Skip to content

Replace subprocess.call() with check_call() in db.py to prevent silent failures#3580

Merged
MoralCode merged 2 commits intochaoss:mainfrom
guptapratykshh:fix/3579-db-cli-check-call
Feb 4, 2026
Merged

Replace subprocess.call() with check_call() in db.py to prevent silent failures#3580
MoralCode merged 2 commits intochaoss:mainfrom
guptapratykshh:fix/3579-db-cli-check-call

Conversation

@guptapratykshh
Copy link
Contributor

@guptapratykshh guptapratykshh commented Jan 15, 2026

Description
I replaced subprocess.call with check_call in the database cli, ensuring that operations stop immediately and report errors if something goes wrong.

This PR fixes #3579

Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

… to prevent silent failures

Signed-off-by: guptapratykshh <pratykshgupta9999@gmail.com>
shlokgilda
shlokgilda previously approved these changes Jan 15, 2026
Copy link
Collaborator

@shlokgilda shlokgilda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Love the short, targeted PR. Added one suggestion to fix a docstring that was incorrect even before this change.

Signed-off-by: guptapratykshh <pratykshgupta9999@gmail.com>
@sgoggins sgoggins added API Related to Augur's metrics API database Related to Augur's unifed data model labels Jan 20, 2026
@sgoggins
Copy link
Member

Waiting on Podman end to end test. The fundamental difference here is really the provision of detailed error messages in check_call both wait for a process to finish ... both return 0 if its all good ... I am not against this change, it seems better, but I think it will provide more information on failure rather than allow failure more quickly ...

@MoralCode MoralCode added this to the v0.93.0 milestone Jan 21, 2026
Copy link
Contributor

@MoralCode MoralCode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_call will raise an exception if something fails.
the way we use call here currently (not storing the results in a variable) effectively means we ignore failures

While this may lead to more crashes, they are crashes that would otherwise be maskes, so knowing about them is better

LGTM

@MoralCode MoralCode merged commit 663dabf into chaoss:main Feb 4, 2026
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API Related to Augur's metrics API database Related to Augur's unifed data model

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Augur DB CLI: create schema should use check_call and fail if there is an error.

4 participants