-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implements the graph command #257
Conversation
@radeksimko I was wondering if you had any feedback on this. You previously helped a teammate of mine with a similar command addition to tfexec. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, the output of the command is stored as just a string, but could potentially be loaded into an object that implements the DOT format.
That sounds like a good idea worth considering prior to tfexec v1, I'll file an issue to track this. Outputting just DOT as string for now seems reasonable!
It is also worth noting that graph
command generally isn't considered "protected worfklow" under the Terraform v1 compatibility guarantees, so in theory the output format is still subject to change.
The PR looks good overall, I just left you a couple of minor suggestions/comments in-line primarily about flags and version gating. Once these are addressed I'm happy to merge this.
t.Fatalf("error running Apply: %s", err) | ||
} | ||
|
||
_, err = tf.Graph(context.Background()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth testing the output here as well? I presume it shouldn't differ between TF versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed minor differences in string value for certain resources in the graph output. Because of this i am validating against one known good string consistent in all outputs. I'm not sure if this satisfies your request or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is much better than before, thank you!
For context, this library is becoming one of the ways we smoke test new Terraform CLI releases, so the subtle differences is actually something we may want to know about going forward. However for now this will do. 👌🏻
Implements the
graph
command for Terraform. At the moment, the output of the command is stored as just a string, but could potentially be loaded into an object that implements the DOT format.