-
Notifications
You must be signed in to change notification settings - Fork 4
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
DB Backup Bug #2853
DB Backup Bug #2853
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2853 +/- ##
===========================================
- Coverage 93.67% 93.62% -0.05%
===========================================
Files 262 265 +3
Lines 6053 6073 +20
Branches 503 510 +7
===========================================
+ Hits 5670 5686 +16
- Misses 287 294 +7
+ Partials 96 93 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
s3_client.upload_file(file_name, bucket, object_name) | ||
print("Uploaded {} to S3:{}{}".format(file_name, bucket, object_name)) | ||
response = s3_client.upload_file(file_name, bucket, object_name) | ||
logger.info(f"S3 upload response: {response}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elipe17 should the response here be None
if its successful? Asking because that's what I'm observing so far.
2024-02-23T12:25:01.07-0500 [APP/PROC/WEB/0] ERR 2024-02-23 17:25:01,070 INFO db_backup.py::upload_file:L152 : S3 upload response: None
2024-02-23T12:25:01.07-0500 [APP/PROC/WEB/0] ERR [2024-02-23 17:25:01,070: INFO/ForkPoolWorker-1] S3 upload response: None
2024-02-23T12:25:01.07-0500 [APP/PROC/WEB/0] ERR 2024-02-23 17:25:01,071 INFO db_backup.py::upload_file:L153 : Uploaded /tmp/backup.pg to s3://...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should return either True or False: https://boto3.amazonaws.com/v1/documentation/api/latest/guide/s3-uploading-files.html. Would like to investigate more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at the source code for our version of the library and it looks like it doesnt return anything. That is a feature added in a more recent version. I will remove the logging of the response since it does not exist for our version of boto3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to figure out why we are getting None instead of False, is it because client is not set?
@elipe17 @raftmsohani please ping me when this is ready again.
|
Sorry @andrew-jameson, I didnt mean to re-request your review. I fat fingered it haha. |
@elipe17 I'm blocked on this one. After changing the minute on the crontab schedule, I'm seeing results like the following in the logs whenever the scheduled task is supposed to start:
|
- update crontabs
@ADPennington I have resolved the issue. One thing you will also have to do (to avoid erroneous error messages) is to delete the periodic task with name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @elipe17 lgtm 🚀 I noted an action item for me to discuss retention policy for backups with data team. cc: @lfrohlich @ttran-hub
Test notes here
Summary of Changes
Pull request closes #2852
How to Test
minute=*/5
. Everything else should be*
's.Deliverables
More details on how deliverables herein are assessed included here.
Deliverable 1: Accepted Features
Checklist of ACs:
lfrohlich
and/oradpennington
confirmed that ACs are met.Deliverable 2: Tested Code
CodeCov Report
comment in PR)CodeCov Report
comment in PR)Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
iamjolly
andttran-hub
using Accessibility Insights reveal any errors introduced in this PR?Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure
Deliverable 8: User Research
Research product(s) clearly articulate(s):