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

Add workflow to calculate source data schema diff #528

Merged
merged 31 commits into from
Nov 6, 2024
Merged

Add workflow to calculate source data schema diff #528

merged 31 commits into from
Nov 6, 2024

Conversation

amishas157
Copy link
Contributor

@amishas157 amishas157 commented Oct 30, 2024 •
edited
Loading

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into two PRs
    otherwise).
  • This PR's title starts with the jira ticket associated with the PR.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or fixes.
  • I've updated the README with the added features, breaking changes, new instructions on how to use the repository.

What

This PR adds github workflow to append source data schema changelog in case if anything changes in schemas dir. If there are no changes changelog won't be appended.

Sample changelog update in

Why

To track schema changes over time.

Known limitations

It tracks changes only at level 1 field and not nested fields.

@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch from f252e82 to e92b9ce Compare October 30, 2024 21:17
@amishas157
Try cloning repo
79121d1 
update

update
@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch 4 times, most recently from e962333 to 09184f8 Compare October 30, 2024 21:35
@amishas157
typo
b1d124b 
Check all edge cases of changelog

test

test

test
@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch from 09184f8 to b1d124b Compare October 30, 2024 21:38
@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch from 99345eb to a33f01e Compare October 30, 2024 22:02
@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch from 4cc574a to 60aeb0a Compare October 30, 2024 22:06
@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch 2 times, most recently from b1fbc79 to 66767d1 Compare October 30, 2024 22:25
@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch from 23cc885 to 9053a28 Compare October 30, 2024 22:28
@amishas157
Copy link
Contributor Author

Test case it adds to an existing file

2024-10-30

Tables Added:

['new_table']

Tables Removed:

['dimAccounts']

Schema Changes:

Table Name Operation Columns
account_signers column_added ledger_sequences
dimOffers column_added another_new_id, new_id
history_assets type_changed asset_code (STRING -> INTEGER), asset_issuer (STRING -> INTEGER)
history_trades column_removed selling_asset_code, selling_asset_type
offers type_changed buying_asset_id (INTEGER -> STRING)
trust_lines column_removed asset_type

2024-01-01

Tables Added:

['new_table']

Tables Removed:

['old_table']

Schema Changes:

Table Name Operation Columns
euro_ohlc type_changed high (TIMESTAMP -> FLOAT)
account_signers column_added ledger_sequences

@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch 3 times, most recently from 04e69ab to 365fc2a Compare October 30, 2024 22:44
@amishas157
revert back all schema changes
77a1e1c 
test

test

figured it out
@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch from 5b47fcc to 77a1e1c Compare October 30, 2024 22:47
@amishas157
Copy link
Contributor Author

Field addition and deletion

2024-10-30

Schema Changes:

Table Name Operation Columns
claimable_balances column_added new_id
dimMarkets column_removed base_issuer

2024-01-01

Tables Added:

['new_table']

Tables Removed:

['old_table']

Schema Changes:

Table Name Operation Columns
euro_ohlc type_changed high (TIMESTAMP -> FLOAT)
account_signers column_added ledger_sequences

@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch from 314743b to 8f732a5 Compare October 30, 2024 23:04
@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch 5 times, most recently from e74bcce to 655deaa Compare November 4, 2024 22:42
@amishas157
push to branch
d4fbe56 
push to branch

push to branch

push to branch

push to branch

push to branch

push to branch
@amishas157 amishas157 force-pushed the patch/update-source-data-schema-changelog branch from 655deaa to d4fbe56 Compare November 4, 2024 22:47
@amishas157
Copy link
Contributor Author

Thanks @chowbao and @sydneynotthecity for the feedback. I have cleaned up the script and modularized it as library. Let me know if changes look good to you.

amishas157
amishas157 commented Nov 4, 2024
View reviewed changes
.github/workflows/update_source_data_schema_changelog.yml
Comment on lines +3 to +4
on:
push:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we trigger on pull request, git push was not working as it was losing the context of branch. Even providing branch name did not help

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh interesting. I think leaving this as push is fine. I can't think of any issues other than it being odd and rerunning every push before a PR is merged

amishas157
amishas157 commented Nov 4, 2024
View reviewed changes
changelog/source_data.md Outdated
@@ -0,0 +1,23 @@
## 2024-11-04:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

File to be deleted before merging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Deleted contents

chowbao
chowbao approved these changes Nov 5, 2024
View reviewed changes
Copy link
Contributor

@chowbao chowbao left a comment

Choose a reason for hiding this comment

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

LGTM

@amishas157 amishas157 merged commit d6ca427 into master Nov 6, 2024
3 checks passed
@sydneynotthecity sydneynotthecity deleted the patch/update-source-data-schema-changelog branch November 21, 2024 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sydneynotthecity sydneynotthecity sydneynotthecity left review comments

@chowbao chowbao chowbao approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@amishas157 @chowbao @sydneynotthecity @actions-user