-
Notifications
You must be signed in to change notification settings - Fork 2
dump MySQL to AWS S3 #688
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
base: main
Are you sure you want to change the base?
dump MySQL to AWS S3 #688
Conversation
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 think given that the test runs are failing that at least one change to the repo is needed!
if 'dbreader' not in config: | ||
print("Please use --rootconfig to specify which configuration file to use") | ||
exit(1) | ||
|
||
if args.dump: |
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.
Well, I mean, the test runs seem to be failing because this condition is not satisfied by the tests.
Ah, yes it would,
…On Thu, Apr 10, 2025 at 10:17 PM Simson L. Garfinkel < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In deploy/app/dbmaint.py
<#688 (comment)>:
> + """Backup to an sqlfile"""
+ if os.path.exists(fname):
+ raise FileExistsError(f"{fname} exists")
+ dbreader = dbfile.DBMySQLAuth.FromConfig(config['dbreader'])
+ cmd = ['mysqldump','-h' + dbreader.host,'-u' + dbreader.user, '-p' + dbreader.password, '--single-transaction', '--no-tablespaces']
+ if all_databases:
+ cmd.append('--all-databases')
+ else:
+ cmd.append(dbreader.database)
+ if fname.startswith('s3://'):
+ print("Dumping to ",fname)
+ with subprocess.Popen(['aws','s3','cp','-',fname], stdin=subprocess.PIPE) as p_s3:
+ subprocess.run(cmd, stdout=p_s3.stdin)
+ print("done")
+ return
+ with open(fname,'w') as outfile:
Won't the return statement in line 378 prevent that?
—
Reply to this email directly, view it on GitHub
<#688 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFQOBYCVSS65SX4OZY3CJD2Y4Q4RAVCNFSM6AAAAAB2WDPA2GVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDONJZGAYDAOBSGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
If you think that adding and else would be more clear, we can do that. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #688 +/- ##
==========================================
- Coverage 70.37% 70.11% -0.27%
==========================================
Files 36 36
Lines 3885 3901 +16
Branches 64 64
==========================================
+ Hits 2734 2735 +1
- Misses 1126 1141 +15
Partials 25 25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# THese need a database config | ||
if 'dbreader' not in config: | ||
# These need a database config | ||
if ('dbreader' not in config) and (args.dump or arts.sqlbackup): | ||
print("Please use --rootconfig to specify which configuration file to use") |
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.
arts.sqlbackup probably needs to be args.sqlbackup
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.
typo in latest comment: arts vs args; see comment
This dumps all of the MySQL databases to AWS S3.
It will be called from a cron job on a daily basis in the production environment.