Page MenuHomePhabricator

Improve output message readabiliy of transfer.py
Closed, ResolvedPublic

Description

The transfer.py is a framework used for database backup and recovery. The output of this framework has unnecessary information about each and every command the cumin runs. Suppressing them would improve the framework output readability/understandability.
The correct way to do this will be by the use of cumin's own method, which is underway here: T212783
A similar issue is solved here like this.
This ticket can be solved by using that solution in the link with a TODO pointing to the above cumin task.

Event Timeline

Change 597069 had a related patch set uploaded (by Privacybatm; owner: Privacybatm):
[operations/software/wmfmariadbpy@master] CuminExecution.py: Improve output message readabiliy of transfer.py

https://gerrit.wikimedia.org/r/597069

I tested this and this can go as is. Question: Should we give more information to stdout? Or when using the verbose mode, or do you think it is ok as it is?

Should we move to use logger for logging messages?

What are your ideas about the ticket subject idea to make it more "user friendly"?

As far as I used this framework, Cumin execution details are not useful to the user at any time.
As a user, I would only be interested to know whether the file is copied successfully or not, which is already present in our framework. And also, we are throwing our own exception messages whenever we fail.

One idea we discussed is packaging (for Debian), it would make this more user friendly.

Cumin execution details are not useful to the user at any time

I 100% agree. I was asking if you thought there was something else we should output like "Opening firewall..." "Stopping MariaDB replication...".

logger is pretty much a standard for logging errors at WMF, while it is not a priority, we should consider later to integrate it (I think for example, cumin already uses it). This will help when integrating transfer.py into other programs. Something to think for later.

I agree also that Debian packaging should have more priority- we should do that next, after the port detection is fully closed. We can have a "version 0.1" release with a debian package. That would be nice.

Change 597069 merged by Jcrespo:
[operations/software/wmfmariadbpy@master] CuminExecution.py: Improve output message readabiliy of transfer.py

https://gerrit.wikimedia.org/r/597069

logger is pretty much a standard for logging errors at WMF, while it is not a priority, we should consider later to integrate it (I think for example, cumin already uses it). This will help when integrating transfer.py into other programs. Something to think for later.

Okay, Sure. I will have a look and understand a little bit more about logging.

I agree also that Debian packaging should have more priority- we should do that next, after the port detection is fully closed. We can have a "version 0.1" release with a debian package. That would be nice.

Yeah! It will be wonderful! :D

@Marostegui I think you'll love what transfer.py looks now (not yet in production, but available on HEAD) thanks to @Privacybatm work (no more garbage output):

$ ./transferpy/transfer.py --no-compress --no-encrypt --no-checksum cumin2001.codfw.wmnet:/home/jynus/test_file2 backup1002.eqiad.wmnet:/home/jynus/
ERROR: The final target path /home/jynus/test_file2 already exists on backup1002.eqiad.wmnet.

$ echo $?
255

$ ./transferpy/transfer.py --no-compress --no-encrypt --no-checksum cumin2001.codfw.wmnet:/home/jynus/test_file3 backup1002.eqiad.wmnet:/home/jynus/
ERROR: The specified source path /home/jynus/test_file3 doesn't exist on cumin2001.codfw.wmnet

$ ./transferpy/transfer.py --no-compress --no-encrypt --no-checksum cumin2001.codfw.wmnet:/home/jynus/test_file backup1002.eqiad.wmnet:/home/jynus/
About to transfer /home/jynus/test_file from cumin2001.codfw.wmnet to ['backup1002.eqiad.wmnet']:['/home/jynus/'] (5 bytes)
5 bytes correctly transferred from cumin2001.codfw.wmnet to backup1002.eqiad.wmnet

Let's document --verbose flag before closing this ticket.

Oh nice work! :-)
I will definitely not miss all the verbosity there hehe

Thanks you @Privacybatm for helping out here!