nbering / terraform-inventory

An Ansible dynamic inventory script to pair with nbering/terraform-provider-ansible.
MIT License
176 stars 52 forks source link

Do not set workspace if it is `default`. #18

Open bigeasy opened 5 years ago

bigeasy commented 5 years ago

Terraform Cloud does not support workspaces. Running terraform.py with a Terraform Cloud backend produces the error 'workspaces not supported'.

With this commit we invoke the command to set the namespace with the -no-color option to receive a plain-text UTF-8 response without terminal color codes. If the stderr response is empty or 'workspaces not supported', we continue with program operation.

nbering commented 5 years ago

Seems reasonable.

I was just thinking, as soon as I saw your PR title, that maybe workspace should default to None, and then skip setting workspace entirely, unless it is specified. That way if this error is encountered it can be fatal, as it probably should be.

bigeasy commented 5 years ago

That's fine too. Maybe have it default to an empty string, just in case someone has a workspace named None.

nbering commented 5 years ago

Maybe have it default to an empty string, just in case someone has a workspace named None.

Ah. I probably wasn't clear enough there. I mean the special Python value, None. Not the string, "None". It's python's equivalent of nil, or null, used when there is no defined value.

bigeasy commented 5 years ago

Sounds good to me. Do you want me to have a go at making it that way?

bigeasy commented 5 years ago

Had a go. Seems to work. Python is not familiar to me, so please critique. If it looks good, I please let me squash commits before merging.

nbering commented 5 years ago

Given a quick eyeball, looks correct to me. I'll give it a little manual test later today... but I think you can go ahead and do your squash if you like.

bigeasy commented 5 years ago

Squashed. Thank you for writing this useful utility.

nbering commented 5 years ago

I run this more or less as a weekend project. I'll get to merging in the next couple days.

Thanks for your contribution! 😄

egadgetjnr commented 3 years ago

@nbering Are we able to get this merged by chance? :)

I'm currently using the updated changes from @bigeasy and can confirm it works.