-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Python3 support and configurable endpoint #4
base: master
Are you sure you want to change the base?
Conversation
## Usage | ||
|
||
Create a new harvest source of type "Socrata" and enter the URL of the Socrata catalog you want to harvest from. The default base url to retrieve catalogues is "https://api.us.socrata.com/api/catalog/v1". You can provide a config object to the harvester to change this base url. For example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document the new configuration option
|
||
try: | ||
from urllib.parse import urlparse | ||
except ImportError: | ||
from urlparse import urlparse | ||
|
||
try: | ||
from json import JSONDecodeError | ||
except ImportError: | ||
from simplejson.scanner import JSONDecodeError | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python 3 imports with python 2 fallbacks
def _set_config(self, config_str): | ||
self.config = {'base_api_endpoint':BASE_API_ENDPOINT} | ||
if config_str: | ||
self.config = json.loads(config_str) | ||
if 'base_api_endpoint' in self.config: | ||
self.base_api_endpoint = self.config['base_api_endpoint'] | ||
|
||
log.debug('Using config: %r', self.config) | ||
|
||
def validate_config(self, config): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allows base_api_endpoint to be configured
@@ -266,8 +298,9 @@ def _page_datasets(domain, batch_number): | |||
_request_datasets_from_socrata(domain, batch_number, | |||
current_offset) | |||
if datasets is None or len(datasets) == 0: | |||
raise StopIteration | |||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a break here rather than raising an error, so imports can continue
This adds a couple of small changes to make the plugin work with python 3. We also needed to be able to change the endpoint the harvester uses (the datasets we wanted were on the .eu version), so
base_api_endpoint
has been added as a configuration option.I can see the repo hasn't been updated for a while! But it only took small changes to get this working, and if we need it someone else might, so hopefully this pull request is useful