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.

Sign up for GitHub

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

Jump to bottom

low-code: Delete now_local macro #25404

Merged
merged 5 commits into from
Apr 26, 2023
Merged

low-code: Delete now_local macro #25404

merged 5 commits into from
Apr 26, 2023

Conversation

girarda
Copy link
Contributor

@girarda girarda commented Apr 21, 2023 •
edited
Loading

What

  • The now_local() macro is unpredictable because it uses the time local to the machine running the sync.
  • We do not recommend connector developers to use this method
  • No existing connectors use the macro
  • We should just delete it to prevent future headaches

How

  • Delete the macro
  • Delete the unit test
  • Remove the macro from the reference docs

Recommended reading order

  1. airbyte-cdk/python/airbyte_cdk/sources/declarative/interpolation/macros.py
  2. airbyte-cdk/python/unit_tests/sources/declarative/interpolation/test_macros.py
  3. airbyte-cdk/python/airbyte_cdk/sources/declarative/declarative_component_schema.yaml

@octavia-squidington-iii octavia-squidington-iii added the CDK Connector Development Kit label Apr 21, 2023
@girarda girarda marked this pull request as ready for review April 21, 2023 17:59
@girarda girarda requested a review from a team as a code owner April 21, 2023 17:59
@girarda girarda requested a review from clnoll April 21, 2023 18:00
@girarda girarda changed the title Delete now_local macro low-code: Delete now_local macro Apr 21, 2023
@girarda girarda mentioned this pull request Apr 21, 2023
Merged
42 tasks
clnoll
clnoll reviewed Apr 21, 2023
View reviewed changes
Copy link
Contributor

@clnoll clnoll left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice! Found a couple more places where references to it can be removed.

@girarda girarda requested review from a team and clnoll April 21, 2023 19:28
@girarda
Copy link
Contributor Author

girarda commented Apr 24, 2023

good catch @clnoll. Removed those references too

@girarda girarda merged commit 250c3b1 into master Apr 26, 2023
@girarda girarda deleted the alex/remove_now_local branch April 26, 2023 03:40
marcosmarxm pushed a commit to natalia-miinto/airbyte that referenced this pull request Jun 8, 2023
@girarda @marcosmarxm
low-code: Delete now_local macro (airbytehq#25404)
b330ab0 
* Delete now_local macro

* Remove from reference docs

* remove example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clnoll clnoll clnoll approved these changes

Assignees
No one assigned
Labels
CDK Connector Development Kit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@girarda @clnoll @octavia-squidington-iii