Skip to content
This repository was archived by the owner on Apr 17, 2026. It is now read-only.

adds strict_db flag to run/cloud/cluster commands#28

Open
ajslone wants to merge 1 commit into
mainfrom
aslone/strict_db_flag
Open

adds strict_db flag to run/cloud/cluster commands#28
ajslone wants to merge 1 commit into
mainfrom
aslone/strict_db_flag

Conversation

@ajslone

@ajslone ajslone commented Jun 24, 2020

Copy link
Copy Markdown
Contributor

Please have a look, this addresses b/158210362, adding a strict_db flag for caliban job execution commands.

This flag will allow the user to request that caliban exit if it cannot connect to the requested database instance specified by the CALIBAN_DB_URL environment variable.

@codecov

codecov Bot commented Jun 24, 2020

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 7.14286% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.65%. Comparing base (28a5ebf) to head (4dc922a).
⚠️ Report is 67 commits behind head on main.

Files with missing lines Patch % Lines
caliban/gke/cli.py 0.00% 8 Missing ⚠️
caliban/main.py 0.00% 2 Missing ⚠️
caliban/cloud/core.py 0.00% 1 Missing ⚠️
caliban/docker.py 0.00% 1 Missing ⚠️
caliban/history/utils.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #28      +/-   ##
==========================================
- Coverage   45.67%   45.65%   -0.03%     
==========================================
  Files          17       17              
  Lines        2728     2736       +8     
==========================================
+ Hits         1246     1249       +3     
- Misses       1482     1487       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread caliban/gke/cli.py
zone=zone,
creds=creds)

return fn(args, cluster=cluster) if cluster else None

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a minor bug I found while debugging this PR.

Comment thread caliban/history/utils.py
try:
return _create_sqa_engine(url=url, echo=echo)

except (OperationalError, OSError) as e:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Another minor issue found while testing this PR.

@ajslone ajslone requested a review from sritchie June 24, 2020 19:58

@sritchie sritchie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you think this makes more sense as a .calibanconfig.json entry? I can see users forgetting the CLI flag. This seems more like a mode you'd want to engage permanently. Thoughts?

@ajslone

ajslone commented Jun 24, 2020

Copy link
Copy Markdown
Contributor Author

That seems like a good assumption, can/should the options exist in both the config and the cli, so the user can override the defaults?

@sritchie

Copy link
Copy Markdown
Contributor

I'm inclined to say that it should only exist in the config. If it exists in the cli too, then we need to have a --nostrict. I had a function a while back that could add these boolean pairs, but then the docs start to balloon. I think I'm okay if this is something you change occasionally. But you've been staring at it longer, @ajslone , let me know your thoughts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants