search

starhawking / python-terrascript

Create Terraform files using Python scripts.
BSD 2-Clause "Simplified" License
516 stars 76 forks source link

remove temp directory after validation #64

Closed maorfr closed 5 years ago

maorfr commented 5 years ago

Great work on this! Thanks a lot! Please accept my humble contribution ;)

mjuenema commented 5 years ago

Hi Maor, welcome and thanks for that. Looking through your change I remembered that this could be done through a context, like in the examples for tempfile which will automatically remove the content. Are you able to rewrite your patch accordingly?

May I add your name to CONTRIBUTORS.md?

Markus

maorfr commented 5 years ago

hey! thanks for replying :) the reason i chose this implementation as it is compatible with python2 as well as python3. what do you think?

mjuenema commented 5 years ago

Terrascript is meant to be compatible with Python 3.3+ only. This was an early design decision after I had many nightmares, making a different project compatible with Python 2 and Python 3. The automated tests are only run against Python 3.3, as configured in .travis.yml. So in regards to the temporary directory I really prefer the Python 3 approach with the context manager.

maorfr commented 5 years ago

i totally agree with the approach. but if i may - from using python-terrascript with python2, except for #65, everything works as expected (at least for the aws bits). would you reconsider? :)

mjuenema commented 5 years ago

There is another big question that needs answering first. Terraform 0.12 will introduce some major changes and I have to decide whether any future Terrascript releases will remain backward compatible with Terraform 0.11.x or whether I start "from scratch", e.g. Terrascript2. And, as a consequence, would a Terrascript2 support Python 3 and Python 2.

In my view the best approach would be to use the Python 2 backports.tempfile module which supports TemporaryDirectory.

maorfr commented 5 years ago

i think i'll go with your first suggestion, and will migrate existing python2 projects to python3. this has to be done either way :)

regarding CONTRIBUTORS.md - feel free, and thank you!

mjuenema commented 5 years ago

For the fun of it I tried a quick test against Python 2.7 but immediately ran into issues that would require more work (https://www.travis-ci.org/mjuenema/python-terrascript/builds/514390932). Python 2 support is definitely something for later.

maorfr commented 5 years ago

for later or for never :) https://pythonclock.org/