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

🐛 Keen destination: fix for timestamp inference for complex types #5973

Merged

Conversation

maciej-nedza
Copy link
Contributor

@maciej-nedza maciej-nedza commented Sep 10, 2021

What

Fixed bug, when destination initialization during syncrhonization was failing for some particular sources which use schema composition for their field's types ( "allOf", "anyOf", "oneOf").
Also race condition causing randomly failing test runs should be finally fixed.

How

Simplified process of timestamp inference, now the map containing streams to cursorfFields mappings doesn't keep track of cursor's type. In process of inference, first the cursor is always tried to be parsed into number, if this fails then it's tried to be parsed into string.

Recommended reading order

  1. x.java
  2. y.python

Pre-merge Checklist

Updating a connector

Community member or Airbyter

  • Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions
  • Connector version bumped like described here

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
  • Credentials added to Github CI. Instructions.
  • /test connector=connectors/<name> command is passing.
  • New Connector version released on Dockerhub by running the /publish command described here

@github-actions github-actions bot added the area/connectors Connector related issues label Sep 10, 2021
@marcosmarxm
Copy link
Member

Thanks @maciej-nedza can you use the Update connector checklist to let us know what steps you are? Do you need any assistance or this is already ready to review?

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Sep 13, 2021
@maciej-nedza
Copy link
Contributor Author

Thanks @maciej-nedza can you use the Update connector checklist to let us know what steps you are? Do you need any assistance or this is already ready to review?

Hey @marcosmarxm , yes, it was already ready for the review. I've updated the checklist and bumped the version.

@@ -88,12 +88,12 @@
- destinationDefinitionId: 8aaf41d0-f6d2-46de-9e79-c9540f828142
name: Keen
dockerRepository: airbyte/destination-keen
dockerImageTag: 0.1.0
dockerImageTag: 0.2.0
Copy link
Contributor Author

@maciej-nedza maciej-nedza Sep 13, 2021

Choose a reason for hiding this comment

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

I've bumped versions here and in the <uuid>.json file, however I don't see updated version number in settings -> Destination after launching the application. Should I do something more ?

Copy link
Contributor

Choose a reason for hiding this comment

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

instances don't autoupdate versions for stability reasons. The user must update it by hand. Versions specified in this/the JSON files are used the very first time Airbyte runs.

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.

Thanks @maciej-nedza !

@@ -88,12 +88,12 @@
- destinationDefinitionId: 8aaf41d0-f6d2-46de-9e79-c9540f828142
name: Keen
dockerRepository: airbyte/destination-keen
dockerImageTag: 0.1.0
dockerImageTag: 0.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

instances don't autoupdate versions for stability reasons. The user must update it by hand. Versions specified in this/the JSON files are used the very first time Airbyte runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/destination/keen connectors/destinations-api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants