Skip to content
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

Add source connector for the PokéAPI #3149

Merged
merged 8 commits into from
May 5, 2021
Merged

Add source connector for the PokéAPI #3149

merged 8 commits into from
May 5, 2021

Conversation

avaidyanatha
Copy link
Contributor

@avaidyanatha avaidyanatha commented Apr 30, 2021

Main Changes

  • This adds a toy source connector for the Open Source Pokémon API found here: https://pokeapi.co/

Endpoints Implemented

  • Currently only the /pokemon endpoint is implemented, allowing the user to retrieve one pokemon's relevant information from it.

Notes/Self-Feedback

  • We can use this as another example of building out a Python source, although it is definitely not fleshed out or meant to be used that seriously.
  • Is there a way we can differentiate between toy/tutorial connectors from the main offerings?
  • No tests have been added yet

@avaidyanatha avaidyanatha changed the title Add toy source connector for the PokéAPI Add source connector for the PokéAPI Apr 30, 2021
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! First connector 4 days in. Good stuff.

url_base = 'https://pokeapi.co/api/v2/'

def __init__(self, name: str, **kwargs):
super().__init__()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best practice is to pass **kwargs to the superclass

Suggested change
super().__init__()
super().__init__(**kwargs)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, done!


def streams(self, config: Mapping[str, Any]) -> List[Stream]:
args = {"name": config["name"]}
return [Pokemon(**args)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bundling arguments into a dict like args is usually useful when you need to pass those params as a group. In this case we can just pass it directly

Suggested change
return [Pokemon(**args)]
return [Pokemon(name=config["name"])]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, done!

"required": ["TODO"],
"additionalProperties": false,
"properties": {
"TODO: This schema defines the configuration required for the source. This usually involves metadata such as database and/or authentication information.": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the contents of ../spec.json should be here, and that one should be deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

"$schema": "http://json-schema.org/draft-07/schema#",
"type": "object",
"properties": {
"name": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I run curl https://pokeapi.co/api/v2/pokemon/pikachu the schema looks way more complicated. This should include it. This allows downstream consumers to know how to format/normalize the data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep makes sense, done!

@michel-tricot
Copy link
Contributor

Told my kids about this connector, they had stars in their eyes!

// configPath points to a config file which matches the spec.json supplied above. secrets/ is gitignored by default, so place your config file
// there (in case it contains any credentials)
// TODO update the config file to contain actual credentials
configPath = "secrets/config.json"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you'll need to change this to sample_files/config.json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed this comment for some reason! Done.

Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant! to publish your connector with Airbyte by default you'll need to:

  1. Add a docs page in docs/integrations/sources/poke-api.md
  2. Add a ref to the doc page in docs/SUMMARY.md
  3. add an entry for this connector in source_definitions.yaml using a newly generated UUID
  4. create a JSON file airbyte-config/init/src/main/resources/config/STANDARD_DESTINATION_DEFINITION with the same info as that in checkin catalogs #3
  5. on this PR, comment /publish connector=connectors/source-pokeapi
  6. If that is green, merge

@avaidyanatha
Copy link
Contributor Author

avaidyanatha commented May 5, 2021

/publish connector=connectors/source-pokeapi

🕑 connectors/source-pokeapi https://github.com/airbytehq/airbyte/actions/runs/811952436
✅ connectors/source-pokeapi https://github.com/airbytehq/airbyte/actions/runs/811952436

@avaidyanatha avaidyanatha merged commit b215733 into master May 5, 2021
@avaidyanatha avaidyanatha deleted the abhi/pokeapi branch May 5, 2021 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants