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

🎉 Source Orb: support enriching ledger entries with numeric properties #10839

Merged
merged 8 commits into from
Mar 10, 2022

Conversation

kgrover
Copy link
Contributor

@kgrover kgrover commented Mar 3, 2022

What

This adds a configuration parameter to the Orb Source connector, numeric_event_properties_keys, which allows the user to list keys whose property values are numeric (as opposed to string-valued). The dynamic schema generation in the connector constructs the schema accordingly.

🐛 This also makes a few schema updates to the existing declared schema, as well as a minor bug fix to output id instead of event_id when outputting an enriched event.

How

When we dynamically generate the JSON schema, enriching credit_ledger_entries.json, we treat the keys passed in for numeric_event_properties_keys separately.

Recommended reading order

  1. spec.json and source.py for the new addition to support numeric properties
  2. Schema files for schema changes

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

This connector should currently be unused, so do not expect any user impact.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

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

Tests

Unit orb_source_unit_tests
Acceptance Screen Shot 2022-03-03 at 9 38 03 AM

@github-actions github-actions bot added the area/connectors Connector related issues label Mar 3, 2022
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Mar 3, 2022
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.

thanks @kgrover

@marcosmarxm
Copy link
Member

@misteryeo can you review the change in spec.json of this contribution?

@marcosmarxm
Copy link
Member

@kgrover can you bump the connector version in airbyte-config/init/src/main/resources and the Dockerfile inside the container folder?
See more info here: https://docs.airbyte.com/connector-development#4.-publish-the-connector

@marcosmarxm marcosmarxm self-assigned this Mar 8, 2022
@lmossman
Copy link
Contributor

lmossman commented Mar 8, 2022

/test connector=connectors/source-orb

Error: No ref found for: kgrover/source-orb-modifications

@lmossman
Copy link
Contributor

lmossman commented Mar 8, 2022

/test connector=connectors/source-orb repo=orbcorp/airbyte

Error: No ref found for: kgrover/source-orb-modifications

@lmossman
Copy link
Contributor

lmossman commented Mar 9, 2022

/test connector=connectors/source-orb repo=orbcorp/airbyte

🕑 connectors/source-orb https://github.com/airbytehq/airbyte/actions/runs/1958952232
❌ connectors/source-orb https://github.com/airbytehq/airbyte/actions/runs/1958952232
🐛

@lmossman
Copy link
Contributor

lmossman commented Mar 9, 2022

Looking at that ^ action, it seems that the test actually passed but the github action just failed to update the above comment with the success status. So I think we can consider that to be a success

@lmossman
Copy link
Contributor

lmossman commented Mar 9, 2022

/publish connector=connectors/source-orb repo=orbcorp/airbyte

🕑 connectors/source-orb https://github.com/airbytehq/airbyte/actions/runs/1959104394
❌ connectors/source-orb https://github.com/airbytehq/airbyte/actions/runs/1959104394

@lmossman
Copy link
Contributor

lmossman commented Mar 9, 2022

/publish connector=connectors/source-orb repo=orbcorp/airbyte

🕑 connectors/source-orb https://github.com/airbytehq/airbyte/actions/runs/1959700950
❌ connectors/source-orb https://github.com/airbytehq/airbyte/actions/runs/1959700950

@lmossman
Copy link
Contributor

lmossman commented Mar 10, 2022

/publish connector=connectors/source-orb repo=orbcorp/airbyte

🕑 connectors/source-orb https://github.com/airbytehq/airbyte/actions/runs/1960393492
✅ connectors/source-orb https://github.com/airbytehq/airbyte/actions/runs/1960393492

Copy link
Contributor

@misteryeo misteryeo left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

It's finished @kgrover. Can you just correct the documetation and run ./gradlew airbyte-config:init:processResources?

@@ -51,6 +51,7 @@ an Orb Account and API Key.
## Changelog

| Version | Date | Pull Request | Subject |
| 0.2.0 | 2022-03-03 | [10839](https://github.com/airbytehq/airbyte/pull/10839) | Support ledger entries with numeric properties + schema fixes
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| 0.2.0 | 2022-03-03 | [10839](https://github.com/airbytehq/airbyte/pull/10839) | Support ledger entries with numeric properties + schema fixes
| 0.1.1 | 2022-03-03 | [10839](https://github.com/airbytehq/airbyte/pull/10839) | Support ledger entries with numeric properties + schema fixes

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 community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants