python: clean job-archive interface code #463
Conversation
Problem: The code for both viewing job records in the flux-accounting database and calculating/updating job usage values is in the same file, job_archive_interface.py. This causes the file to be very large and hard to read; it would be best to separate these two different features (viewing job records & calculating usage) into two, smaller files. Move the code for viewing job records into a new file called jobs_table_subcommands.py. Adjust the code in job_archive_interface.py, the flux-accounting service, and t1006_job_archive.py to account for the newly-moved code.
There was a problem hiding this comment.
Looks good to me. It looks like a lot of the python code in this repo has function descriptions like
# description
def function():
pass
or
def function():
# description
pass
but I think the usual Python style is
def function():
"""description"""
I noticed it in some of the code in this PR and then saw that it was pretty consistent across the repo.
| @@ -88,7 +88,9 @@ def write_records_to_file(job_records, output_file): | |||
| ) | |||
|
|
|||
|
|
|||
| def fetch_job_records(job_records): | |||
| # Convert the results of a query to the jobs table to a readable string | |||
| # that can either be output to stdout or written to a file. | |||
There was a problem hiding this comment.
This is the style police 👮♂️ 🚔 you have violated the law
There was a problem hiding this comment.
👐 i swear i'm innocent!
I will change the style of the function descriptions that I've created in this PR. If it's okay, I'll open a separate one to fix the descriptions in the rest of the repo.
Problem: Some of the functions moved over from job_archive_interface could use better names, function definitions, and descriptions. Change the names of a couple of the functions to better reflect what they do. Add a couple function descriptions where appropriate. Add a small helper function to filter queried jobs by user before filtering those jobs by bank. Adjust the flux-accounting service and t1006_job_archive.py to account for the name changes to a couple of the functions.
Problem: job_archive_interface.py could use a better filename since the contents of the file have changed. The code in this file primarily focuses on calculating and updating job usage values for associations in the flux-accounting database. Rename job_archive_interface --> job_usage_calculation. Adjust the various files that list and import this file to account for the name change.
Problem: There is a leftover print() statement in one of the unit tests in t1006_job_archive.py that is not needed. Remove it.
cmoussa1
force-pushed
the
clean.job-archive-interface
branch
from
ff1c091 to
39f1a8a
Compare
|
Thanks for reviewing @jameshcorbett. I've pushed up a fix to the function descriptions I wrote in this PR. I'll open up an issue to step through the Python code in this repo and update the rest of the function descriptions. Setting MWP here |
Problem
The code for both viewing job records in the flux-accounting database and calculating/updating job usage values is in the same file,
job_archive_interface.py. This causes the file to be very large and hard to read; it would be best to separate these two different features (viewing job records & calculating usage) into two, smaller files, as well as clean up some of the existing code for viewing job records.This PR moves the code for viewing job records into a new file called
jobs_table_subcommands.py. It also makes some slight improvements to the function names and adds some descriptions to a couple of those moved functions to better represent what they are doing.The code in
job_archive_interface.py(renamed tojob_usage_calculation.py), the flux-accounting service, and unit tests are adjusted to account for this moved code.