update python cdk tutorial with updates to exchange rates api #12427
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
area/connectors
brianjlai
commented
Apr 28, 2022
| @@ -14,14 +14,16 @@ | |||
|
|
|||
|
|
|||
| class ExchangeRates(HttpStream): | |||
| url_base = "https://api.exchangeratesapi.io/" | |||
| url_base = "http://api.exchangeratesapi.io/" | |||
There was a problem hiding this comment.
Exchange Rates only allows free tier to use http not https
brianjlai
commented
Apr 28, 2022
| # The api requires that we include the base currency as a query param so we do that in this method | ||
| return {'base': self.base} | ||
| # The api requires that we include access_key as a query param so we do that in this method | ||
| return {'access_key': self.access_key} |
There was a problem hiding this comment.
Free tier users no longer have the ability to specify a base. Leaving it in results in 400 errors. Although its confusing from a tutorial standpoint to allow for base, base is used in earlier sections to demonstrate invalid configs so I think we should not completely remove it from the tutorial since it still serves a purpose, even if not in the outbound req
girarda
approved these changes
Apr 28, 2022
| @@ -38,8 +40,8 @@ def request_params( | |||
| stream_slice: Mapping[str, Any] = None, | |||
| next_page_token: Mapping[str, Any] = None, | |||
| ) -> MutableMapping[str, Any]: | |||
| # The api requires that we include the base currency as a query param so we do that in this method | |||
| return {"base": self.base} | |||
| # The api requires that we include access_key as a query param so we do that in this method ADD THIS | |||
There was a problem hiding this comment.
the ADD THIS. comment is a little confusing. what needs to be added?
There was a problem hiding this comment.
nice catch! I forgot to delete that as i was marking what code to add to the tutorial. will delete this
Amruta-Ranade
approved these changes
Apr 29, 2022
suhomud
pushed a commit
that referenced
this pull request
May 23, 2022
* update python cdk tutorial with updates to exchange rates api * remove extra comment in source.py tutorial sample code
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue: #12249
What
The Exchange Rates API that we use as part of the "Python CDK: Creating a HTTP API Source" tutorial has changed quite a bit in the past few months. In particular, the introduction of required API access keys and some new limitations on what Free tier users can utilize has made out instructions fall out of sync.
How
Most of the changes to the API can all be reasoned about with some experimentation, but in order to reduce the friction of people going through the tutorial, I've updated the steps and code snippets to incorporate changes to the API. I've also updated some of the connector code to align with the new changes.