Conversation
…atabase.
Refactors the main loop into an emit_jobs() function to handle the
output dispatching, and conditionally check for the presence of the DB
using the machine attribute from the configuration.
Key aspects of the change:
1. Scope/Binding Issues Avoided:
Because averages and num_jobs were previously only initialized
dynamically under the if args.average: block, they would cause an
UnboundLocalError or NameError in emit_jobs() if not properly
scoped. I've ensured these are initialized before emit_jobs is
defined so the nonlocal directive bindings are stable regardless of
the CLI arguments.
2. Simplified Output Dispatching:
The repetitive job output logic is now cleanly housed inside
emit_jobs(jobs_iter).
3. Database Integration with Scan Fallback:
If config.machine is set and db_available(machine) evaluates to
True, it fetches records from the DB using
db_get_records. Otherwise, it logs a warning (if machine is
present) and predictably falls back to scanning the PBS logs.
Closed
Collaborator
|
Looks good to me. Having machine selection doable via an option would be a nice extension, but it's moot until the DB server is robust and in production. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces the ability for
qhistto optionally query job records from thehpc-usage-queriesjobhistdatabase module instead of relying exclusively on flat-file PBS log scanning.To support this, an optional
jobhist-dbdependency has been added topyproject.tomlthat installs the required database integrations. Inside the main application, the core output and formatting logic has been refactored into a reusableemit_formatted_jobs()helper function. The tool now dynamically checks if the database module is installed and available for the target machine; if so, it fetches the records directly using the new database API. If the database is unavailable or the import fails,qhistwill gracefully fall back to the original log file scanning behavior, emitting a warning.Additionally, the PR adds a
.env.examplefile to document the available database configuration settings (supporting both SQLite and PostgreSQL backends) and updated.gitignoreto ensure local.envcredentials are kept out of source control. The target machine can now be explicitly overridden via theQHIST_MACHINEenvironment variable.Finally, this update includes minor variable scoping fixes (nonlocal bindings for statistics tracking) to support the refactored formatting loop.
Open issues or questions:
.envis not necessarily required; but some configuration/passthrough is needed for Postgres server connection setupmachinecould optionally be exposed through CLI arguments. This is not done currently.