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

Fix: Tutorial config-based invalid keywords #20071 #20078

Merged
merged 3 commits into from
Dec 8, 2022

Conversation

juweins
Copy link
Contributor

@juweins juweins commented Dec 5, 2022

What

The yaml reference files in the tutorial for config-based connectors provided wrong keywords. Therefore, a user could not follow the example properly.

How

Adjusted the error-leading keywords on all occurences in this tutorial:

spec: 
  documentationUrl: https://docs.airbyte.io/integrations/sources/exchangeratesapi
  connectionSpecification:

This PR also includes the related Issue #20047 , which also gets resolved.

The PR will also resolve #18663. (Duplicate)

🚨 User Impact 🚨

Minor.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

Updating a connector ### Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub and connector version bumped by running the /publish command described here
Connector Generator
  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • Documentation which references the generator is updated as needed

Tests

Manually worked through the tutorial to ensure correctness.

Comment from Contributor

I am not able to rename commit 0f35da5 to reference #20047 instead of misleading issue number. Maybe you can fix this.

Added the suggested solution from issuer.
Appended the suggested +00:00 into another occurence. Datetime format now corresponds to standards defined in ISO-8601.
@octavia-squidington-iv octavia-squidington-iv added area/documentation Improvements or additions to documentation community labels Dec 5, 2022
Comment on lines +14 to +15
documentation_url: https://docs.airbyte.io/integrations/sources/exchangeratesapi
connection_specification:
Copy link
Member

Choose a reason for hiding this comment

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

Why are you changing this? It doesn't look correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I came accross this error by validating issue #20047 . Strictly following the tutorial example, both keywords in the yaml snippets seem to be incorrect. Changing them to the proposed naming resolves the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is issue has been stated in #18663

Copy link
Member

Choose a reason for hiding this comment

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

@juweins juweins requested a review from marcosmarxm December 7, 2022 09:11
Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

@brianjlai can you make a quick review of this change?

Copy link
Contributor

@brianjlai brianjlai left a comment

Choose a reason for hiding this comment

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

changes look good to me! thanks for the contribution 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Improvements or additions to documentation community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Low-code CDK: Tutorial Bug
4 participants