Implement basic ExchangeRate API using Santa. #2942
Conversation
| return None | ||
|
|
||
| def path(self, **kwargs) -> str: | ||
| return "api/latest" |
There was a problem hiding this comment.
Maybe we shouldn't query for latest in this connector but use the date instead. If we do so then the date can be our state.
There was a problem hiding this comment.
yup, we can also use stream_state to determine what the date should be. I think this is also a better fit for batching. We should create this PR once #2924 is merged (or rebase it and build on top of it)
There was a problem hiding this comment.
yup! was slowly inching my way through python and santa land to this end state.
This can be done as a constructor parameter for the stream class
this is provided to We'll probably want to implement this using the batching concept introduced in #2924 |
| @@ -139,13 +139,16 @@ def _read_incremental( | |||
| logger.info(f"Setting state of {stream_name} stream to {stream_state.get(stream_name)}") | |||
|
|
|||
| checkpoint_interval = stream_instance.state_checkpoint_interval | |||
| batches = stream_instance.stream_slices( | |||
| slices = stream_instance.stream_slices( | |||
There was a problem hiding this comment.
the changes in this file seems like missed changes from the earlier refactor
| moduleDirectory 'source_exchange_rates' | ||
| } | ||
|
|
||
| airbyteStandardSourceTestFile { |
There was a problem hiding this comment.
Following https://docs.airbyte.io/contributing-to-airbyte/building-new-connector/testing-connectors, is this enough to correctly set tests up?
| moduleDirectory 'source_exchange_rates' | ||
| } | ||
|
|
||
| airbyteStandardSourceTestFile { |
airbyte-integrations/connectors/source-exchange-rates/source_exchange_rates/source.py
Show resolved
Hide resolved
|
|
||
| def parse_response(self, response: requests.Response, **kwargs) -> Iterable[Mapping]: | ||
| response_json = response.json() | ||
| yield from [response_json] |
There was a problem hiding this comment.
can also do yield response.json()
There was a problem hiding this comment.
have you verified that a json is returned also on the weekends when there are no exchange rates?
There was a problem hiding this comment.
good catch on yield
it returns Fridays, which is alright but strange. edited to exclude weekends. nice catch.
| self._base = base | ||
| self._start_date = start_date | ||
|
|
||
| def next_page_token(self, response: requests.Response) -> Optional[Mapping[str, Any]]: |
There was a problem hiding this comment.
no need to override this method, returns None by default
There was a problem hiding this comment.
I don't think that's true? https://github.com/airbytehq/airbyte/blob/master/airbyte-integrations/bases/base-python/base_python/sdk/streams/http.py#L69
should we make this return None?
There was a problem hiding this comment.
oh nvm my mistake! I think I made it abstract to "force" the user to take a stance on pagination. LGTM then
airbyte-integrations/connectors/source-exchange-rates/source_exchange_rates/source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-exchange-rates/source_exchange_rates/source.py
Outdated
Show resolved
Hide resolved
|
/publish connector=bases/base-python |
|
/publish connector=bases/base-python
|
|
/publish connector=bases/base-python
|
What
Take Santa for a test drive by implementing the exchange rate api on the
latestroute using Santa. I didn't see a good way to implement incremental on this, since the cursor is part of the path, and I don't think we have a good way of 1) passing the start_date from the config 2) the state from the last record. Might be missing something so happy to talk through.Since I was focused on understanding and evaluating the framework, I did not get to write tests. Honestly was also not sure how to think/structure tests for this. I tested this manually on the command line using the
main_dev.pyfile -python main_dev.py read --config sample_files/config.json --catalog sample_files/configured_catalog.jsonHow
See the files.
Recommended reading order
source.pyspec.json