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

[ch] Dynamo replicator: add try catch, script for local testing #5715

Merged
merged 8 commits into from
Sep 27, 2024
Merged

[ch] Dynamo replicator: add try catch, script for local testing #5715

merged 8 commits into from
Sep 27, 2024

Conversation

clee2000
Copy link
Contributor

@clee2000 clee2000 commented Sep 25, 2024 •
edited
Loading

  • Add try catch to catch errors when inserting
  • Add simple script so you can see the insert query (and test in console). Adds sample json with sample record for workflow_job (only 1 though)

Tested by

  • Running new test_lambda_function script to see what the query would be, changed the database to fortesting, ran in console
  • Making new lambda, testing that insert still works for workflow_job, tested that errors get sent to gen_errors table in Test #5714

@vercel Vercel
Copy link

vercel bot commented Sep 25, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
torchci ⬜️ Ignored (Inspect) Visit Preview Sep 27, 2024 4:20pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 25, 2024
@clee2000 clee2000 requested a review from a team September 25, 2024 20:34
@clee2000 clee2000 marked this pull request as ready for review September 25, 2024 20:34
huydhn
huydhn reviewed Sep 27, 2024
View reviewed changes
aws/lambda/clickhouse-replicator-dynamo/test_lambda_function.py Outdated
# try it out in the console. Change the database to fortesting to check.
with open(Path(__file__).parent / "sample_workflow_job.json") as f:
event = json.load(f)
handle_event(event, dry_run=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could dump the query string here in dry-run mode for testing, i.e. writing it to query.sql for example and check against the expected value in another file, i.e. sample_query.sql

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, but its a bit jank

huydhn
huydhn approved these changes Sep 27, 2024
View reviewed changes
Copy link
Contributor

@huydhn huydhn left a comment

Choose a reason for hiding this comment

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

LGTM! A small point about checking the query that is print to console in dry-run mode against its expected value for testing

@clee2000 clee2000 merged commit 84f2e16 into main Sep 27, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@huydhn huydhn huydhn approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@clee2000 @huydhn @facebook-github-bot