backend: drop dvc-specific working-dir behavior #17
Conversation
- drop make_tf contextmanager - use single working_dir per TerraformBackend instance (instead of nesting by resource name)
Codecov Report
@@ Coverage Diff @@
## master #17 +/- ##
==========================================
- Coverage 79.60% 79.25% -0.35%
==========================================
Files 5 5
Lines 201 188 -13
==========================================
- Hits 160 149 -11
+ Misses 41 39 -2
Continue to review full report at Codecov.
|
| pass | ||
|
|
||
|
|
||
| class TerraformBackend: |
There was a problem hiding this comment.
Feels like we should maybe rename this? The base/terraform backend naming is another leftover from dvc-machine and doesn't really have any meaning in TPI.
| tf_file = os.path.join(self.working_dir, "main.tf.json") | ||
| with open(tf_file, "w", encoding="utf-8") as fobj: | ||
| fobj.write(render_json(name=name, **config, indent=2)) | ||
| self.tf.cmd("init") | ||
| self.tf.cmd("apply", auto_approve=IsFlagged) |
There was a problem hiding this comment.
The one thing to note now is that since the init'ed working_dir is always used (instead of separate dirs per resource name), this class is currently only useful for managing a single iterative-machine resource at a time (since we just rewrite the entire main.tf.json any time a config change is made).
A proper TPI release should parse the entire existing main.tf.json and only add/modify/delete the appropriate named resource entries so that you can actually manage multiple resources at a time.
casperdcl
left a comment
casperdcl
left a comment
There was a problem hiding this comment.
The 2 comments will probably have to be addressed at some point. Happy to merge as-is though & iterate, up to you.
|
I'd prefer merging this as-is for now, since the existing behavior is sufficient for dvc-machine, and since I'm not sure what the CML team's plans are for tpi as a standalone tool in general. Basically all of the existing create/destroy functionality is aimed at dealing with single resources - there is no current analog for configuring a If tpi as a standalone tool is intended to behave similarly to dvc-machine and CML (and manage individual runners separately), it seems like the tpi CLI will need to end up doing some kind of separation by directories (like dvc-machine), since AFAIK that's the only way to manage groups of resources (or individual resources) separately using the terraform CLI. (Meaning the changes I mentioned would not seem to be high priority). If tpi as a standalone tool is intended to be more of a full-featured terraform replacement, then finer-grained support for plan/apply (and multiple resources per-plan) would be necessary (but it's not a DVC requirement at this time, and I don't expect that it would be in the future). |
|
@casperdcl can we push a release w/this change, it's currently blocking DVC |
|
sure |
|
/tag v2.0.0 fc779c4 |
3 participants
Will close #15
nesting by resource name)