🎉 Destination S3 & GCS: support additional properties by tuliren · Pull Request #7288 · airbytehq/airbyte (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation22 Commits19 Checks0 Files changed

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

[ Show hidden characters]({{ revealButtonHref }})

What

Resolves Destination S3 to Parquet/Avro: Handle empty objects with additionalProperties=true #4608.

How

Use patched Json / Avro converter from https://github.com/airbytehq/json-avro-converter.

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/ command is passing.
New Connector version released on Dockerhub by running the /publish command described here

tuliren temporarily deployed to more-secrets

October 22, 2021 20:42

Inactive

tuliren changed the title~~Support additional properties in json schema~~ 🎉 Destination S3 & GCS: support additional properties

Oct 22, 2021

tuliren temporarily deployed to more-secrets

October 22, 2021 21:54

Inactive

tuliren temporarily deployed to more-secrets

October 22, 2021 23:12

Inactive

tuliren temporarily deployed to more-secrets

October 23, 2021 08:26

Inactive

tuliren temporarily deployed to more-secrets

October 23, 2021 10:25

Inactive

jrhizor temporarily deployed to more-secrets

October 23, 2021 10:27

Inactive

Contributor Author

tuliren commented

Oct 23, 2021

•

edited by github-actionsbot

tuliren temporarily deployed to more-secrets

October 23, 2021 10:41

Inactive

jrhizor temporarily deployed to more-secrets

October 23, 2021 10:42

Inactive

Contributor Author

tuliren commented

Oct 23, 2021

•

edited by github-actionsbot

tuliren temporarily deployed to more-secrets

October 23, 2021 10:45

Inactive

jrhizor temporarily deployed to more-secrets

October 23, 2021 10:47

Inactive

tuliren marked this pull request as ready for review

October 23, 2021 11:11

tuliren temporarily deployed to more-secrets

October 23, 2021 11:12

Inactive

tuliren temporarily deployed to more-secrets

October 23, 2021 11:15

Inactive

tuliren temporarily deployed to more-secrets

October 23, 2021 15:21

Inactive

Contributor Author

tuliren commented

Oct 23, 2021

•

edited by github-actionsbot

Contributor Author

tuliren commented

Oct 23, 2021

•

edited by github-actionsbot

tuliren temporarily deployed to more-secrets

October 23, 2021 19:34

Inactive

jrhizor temporarily deployed to more-secrets

October 23, 2021 19:35

Inactive

jrhizor temporarily deployed to more-secrets

October 23, 2021 19:35

Inactive

Choose a reason for hiding this comment

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

if I'm reading this file correctly, the json schema doesn't actually need to have additionalProperties=true for this column to be added/handled correctly, is that right? That seems like a good idea because it makes it resilient to undocumented/unannounced schema changes

Choose a reason for hiding this comment

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

Right. Most schemas do not specify additionalProperties=true and many of them include additional fields anyway. Eventually, I think we should handle all invalid cases. The S3 destination does not judge. It only hoards!

tuliren temporarily deployed to more-secrets

November 3, 2021 08:16

Inactive