Pr country search #5
Conversation
| @@ -1,4 +1,4 @@ | |||
| VTOOLS_API="https://events.vtools.ieee.org/RST/events/api/public/<vtools_event_api_version>/" | |||
| CALENDAR_ID="<google_calendar_collab_email>" | |||
| COUNTRY_ID=<vtools_events_country_id> | |||
| FILTER_MODE="COUNTRY" | |||
| COUNTRY_CODE="<ISO_3166-1_alpha-2_code>" | |||
There was a problem hiding this comment.
This implementation is making me thing we should instead look for arguments instead of environment variables. I.e., running the script with -c <country_id> or --country-code <iso-code>.
lib/calendar_api.py
Outdated
There was a problem hiding this comment.
This change looks excellent. However, it doesn't belong to this PR (the changes are scoped to Country search instead). Please create a PR with this changes instead
lib/vtools_fetcher.py
Outdated
There was a problem hiding this comment.
Same comments as above. This should not be a PR for refactors, but instead a PR for country search.
| vtf = EventFetcher(base_url=os.getenv('VTOOLS_API')) | ||
|
|
||
| country_code = os.getenv('COUNTRY_CODE') | ||
|
|
||
| # TODO: Create exception class for this error | ||
| if country_code is None: | ||
| raise Exception() | ||
|
|
||
| country_code = country_code.upper() | ||
| country_id = None | ||
| countries = vtf.fetch_countries() | ||
| for country in countries: | ||
| id = country['id'] | ||
| abbreviation = country['attributes']['abbreviation'] | ||
| if abbreviation == country_code: | ||
| country_id = id | ||
| break | ||
|
|
||
| # TODO: Create exception class for this error | ||
| if country_id is None: | ||
| raise Exception() |
There was a problem hiding this comment.
What I don't like about this is the inability to use the country_id instead. I would prefer to have COUNTRY_CODE as an override argument for it.
Or even to have a cached variable with country id when looking for the country code.
| vtf = EventFetcher(base_url=os.getenv('VTOOLS_API')) | ||
|
|
||
| country_code = os.getenv('COUNTRY_CODE') | ||
|
|
||
| # TODO: Create exception class for this error | ||
| if country_code is None: | ||
| raise Exception() | ||
|
|
||
| country_code = country_code.upper() | ||
| country_id = None | ||
| countries = vtf.fetch_countries() | ||
| for country in countries: | ||
| id = country['id'] | ||
| abbreviation = country['attributes']['abbreviation'] | ||
| if abbreviation == country_code: | ||
| country_id = id | ||
| break | ||
|
|
||
| # TODO: Create exception class for this error | ||
| if country_id is None: | ||
| raise Exception() |
There was a problem hiding this comment.
What I don't like about this is the inability to use the country_id instead. I would prefer to have COUNTRY_CODE as an override argument for it.
Implementation of the country search strategy by its ISO code.
The environment variables are reincorporated into the main function, so constructors are refactored to support the step in that method
The data retrieval functions of the vTools Events API are transformed into an object