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

[DE-2698] Update/remove any of #1

Merged
merged 4 commits into from
Nov 12, 2022
Merged

[DE-2698] Update/remove any of #1

merged 4 commits into from
Nov 12, 2022

Conversation

treeforest456
Copy link

seems like anyOf type is skipped by target-snowflake and it's a known issue here. i forked our tap-anvil and updated the schema, ran it locally and got createdAt/updatedAt in

select
updatedat,
TO_TIMESTAMP_NTZ(updatedat)
from
raw_stg.anvil.submissions

maybe we just make it string in raw and parse it later in stg

@treeforest456 treeforest456 requested a review from a team November 11, 2022 19:54
@jbishopVouch
Copy link

Is there somewhere we can document this, even just a code comment?

Copy link

@avyfain avyfain left a comment

Choose a reason for hiding this comment

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

Since the update is in JSON we can't really leave comments in line, but let's make a comment about it in the README? can't think of a different spot that makes more sense

@avyfain
Copy link

avyfain commented Nov 11, 2022

maybe under https://github.com/svinstech/tap-anvil/blob/main/tap_anvil/client.py#L16 ?

@@ -1,6 +1,6 @@
[tool.poetry]
name = "tap-anvil"
version = "0.1.5"
version = "0.1.6"
Copy link
Author

Choose a reason for hiding this comment

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

not sure if this is the only place that need to update? is there any other actions needed?

Copy link

Choose a reason for hiding this comment

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

I think that's it, we're installing from main AFAIK anyway, no?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

ah so we do need to update in airflow too and create the tag/release

Copy link
Author

Choose a reason for hiding this comment

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

so that's after we merge this and then click on the create a new release here?
Screenshot 2022-11-11 at 3 55 49 PM

Copy link

Choose a reason for hiding this comment

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

yeah!

@treeforest456 treeforest456 changed the title Update/remove any of [DE-2698] Update/remove any of Nov 12, 2022
@treeforest456 treeforest456 merged commit e8ec61e into main Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants