Skip to content

Conversation

@maorfr
Copy link
Contributor

@maorfr maorfr commented Mar 31, 2019

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

@mjuenema
Copy link
Collaborator

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
Copy link
Contributor Author

maorfr commented Apr 1, 2019

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
Copy link
Collaborator

mjuenema commented Apr 1, 2019

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
Copy link
Contributor Author

maorfr commented Apr 1, 2019

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
Copy link
Collaborator

mjuenema commented Apr 1, 2019

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
Copy link
Contributor Author

maorfr commented Apr 2, 2019

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
Copy link
Collaborator

mjuenema commented Apr 2, 2019

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
Copy link
Contributor Author

maorfr commented Apr 2, 2019

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

@mjuenema mjuenema merged commit e80c16a into starhawking:develop Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants