Skip to content

Blevitan dev - Retry Logic - #126

Open
blevitan516 wants to merge 18 commits into
mainfrom
blevitan_dev
Open

blevitan516 wants to merge 18 commits into
mainfrom
blevitan_dev

Conversation

@blevitan516

@blevitan516 blevitan516 commented Sep 23, 2023

Copy link
Copy Markdown
Collaborator

Here is a pull request for the retry logic. I haven't done any testing yet and probably won't have a chance to revisit this for a few weeks, but I wanted to get the code in your hands if you have time to review.

@caiotomazelli caiotomazelli left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall LGTM, but noted some potential problems and nits below.

PTAL and rebase with the latest changes so that I can take a new look please!

Comment thread dags/retries.py Outdated
Comment thread dags/retries.py Outdated
Comment thread docker-compose.yaml
Comment thread dags/utils/run_result.py
Comment thread dags/sources/retry.py Outdated
Comment thread dags/sources/retry.py
Comment thread dags/retries.py Outdated
Comment thread dags/utils/dag_utils.py Outdated
Comment thread dags/utils/dag_utils.py
run_result += target_destination.send_data(data, dry_run)
offset += batch_size
## TESTING ONLY ######################################################
# TODO(blevitan): REMOVE this line before merging BLEVITAN_DEV branch into MAIN.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove this line

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

will remove when done testing on rebase.

Comment thread dags/utils/dag_utils.py Outdated
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.

2 participants