google / caliban

Research workflows made easy, locally and in the Cloud.
https://caliban.readthedocs.io
Apache License 2.0
495 stars 67 forks source link

Project restructure to make it easier to implement more backends #32

Closed sritchie closed 4 years ago

sritchie commented 4 years ago

This PR refactors:

The tests are also split accordingly.

I snuck in ONE bugfix that appeared during testing. I moved our method of generating credentials from gcloud auth login over to caliban.util.auth so that I could reuse it in the caliban history API. We had one spot where we were calling discovery.build and NOT passing any credentials, which threw an error when the user didn't have GOOGLE_APPLICATION_CREDENTIALS specified.

I also removed unused imports and sorted imports wherever I could.

Why?

The project needed it; specifically this is going to make it easier to increase test coverage, and to get the .calibanconfig.json code working with a proper schema, so that we can start to add options there.

I really want to make the platforms pluggable, and sorting them out into separate modules is the first step.

codecov[bot] commented 4 years ago

Codecov Report

Merging #32 into master will increase coverage by 1.04%. The diff coverage is 60.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #32      +/-   ##
==========================================
+ Coverage   46.52%   47.57%   +1.04%     
==========================================
  Files          17       26       +9     
  Lines        2764     2825      +61     
==========================================
+ Hits         1286     1344      +58     
- Misses       1478     1481       +3     
Impacted Files Coverage Δ
caliban/platform/cloud/types.py 89.47% <ø> (ø)
caliban/platform/gke/types.py 63.33% <ø> (ø)
caliban/platform/run.py 25.71% <25.71%> (ø)
caliban/platform/gke/cluster.py 30.05% <30.00%> (ø)
caliban/platform/shell.py 34.48% <34.48%> (ø)
caliban/docker/build.py 31.49% <40.00%> (ø)
caliban/platform/gke/cli.py 21.83% <42.85%> (ø)
caliban/platform/notebook.py 45.00% <45.00%> (ø)
caliban/util/argparse.py 47.36% <47.36%> (ø)
caliban/config/__init__.py 47.94% <47.94%> (ø)
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 85c0bc3...3329f46. Read the comment docs.

sritchie commented 4 years ago

@ajslone , I've tested caliban {shell, run, cloud}, and caliban status just with the basic command, no options. Would you mind helping me test the GKE code, and fixing any busted imports that I've forgotten?

Thank you!