Skip to content
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

Options to always generate a new config file or ignore an existing one #47

Closed
wants to merge 1 commit into from

Conversation

thestuckster
Copy link

Motivation

We've recently been using tflocal and found it frustrating while debugging that we would have to constantly manually delete the existing override file before running tflocal commands.

This PR adds a few lines and extra options to help alleviate this pain point.

Changes

  • adds ALWAYS_GENERATE env variable to determine whether to delete the existing config file and generate a new one each run
  • adds IGNORE_EXISTING_FILE env variable to determine whether to raise an exception or not if an existing config file exists
  • break the config file writing functionality into its own method for DRY
  • update README documentation with the new options

bradengroom
bradengroom approved these changes Feb 16, 2024
Copy link

@rice2007 rice2007 left a comment

Choose a reason for hiding this comment

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

Mid

@lakkeger
Copy link
Contributor

lakkeger commented Feb 19, 2024

Hi @thestuckster,
Thank you for your PR and to address this issue, it seems like a great idea.

Kudos to break the config file writing functionality into its own function. 👍

On the other hand, we can't accept this PR in its current form for multiple reasons:

  • to ignore an already existing file for manual config in my opinion shouldn't be covered by tflocal as that way tflocal losing it's purpose completely. This can be done natively by running terraform.
  • hence the above to control the throw/not throw exception loses its purpose
  • to delete/overwrite a generated file at the start of a tflocal run might be troublesome as it could be a file that was left there on purpose to configure the run manually without tflocal, so an exception is something in my opinion we want to see there as a safety measure. (an env variable could be set and forgotten easily if you work on multiple projects from the same terminal)

However to resolve the situation I would propose the following:

  • implement a DRY_RUN env variable to control the generate behaviour and terminate the run without invoke terraform and deleting the file
  • if such file exists during a dry run the user should be prompted so he/she can decide if he/she wants to override the file or not, this way there are no accidents
  • add a test on the feature to maintain/improve test coverage

If you add the above proposed changes in your PR we're happy to merge it and release it.

@lakkeger lakkeger self-requested a review February 19, 2024 15:34
@lakkeger lakkeger added the enhancement New feature or request label Feb 19, 2024
@thestuckster
Copy link
Author

@lakkeger Thanks so much for the quick feedback!

I see where you are coming from and will work on your suggestions

@lakkeger
Copy link
Contributor

lakkeger commented Mar 5, 2024

The above PR is obsolete, closing it as the requested feature is implemented in #50.

@lakkeger lakkeger closed this Mar 5, 2024
@lakkeger lakkeger added the invalid This doesn't seem right label Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants