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

Empty env keys aren't backwards compatible #42

Closed
AngelOnFira opened this issue Feb 15, 2022 · 10 comments · Fixed by #44
Closed

Empty env keys aren't backwards compatible #42

AngelOnFira opened this issue Feb 15, 2022 · 10 comments · Fixed by #44
Labels
bug Something isn't working

Comments

@AngelOnFira
Copy link
Member

Well, somehow this broke a project CI/CD pipeline I am working on. It was earlier configured to use empty Email Host Name, hence it's currently failing with the error **Empty env key**. Any help would be appreciated

Originally posted by @mannyanebi in #40 (comment)

@AngelOnFira
Copy link
Member Author

So this was fixed from #29, where I agree that it would be better for CI to fail if a key is empty (it can't be found in secrets potentially). @mannyanebi it sounds like your .env file is suppose to have an empty value?

@AngelOnFira AngelOnFira added the bug Something isn't working label Feb 15, 2022
@AngelOnFira
Copy link
Member Author

I will also add, if you use SpicyPizza/[email protected] for the action instead of @main/@v1 then it should work fine 👍

@mannyanebi
Copy link

Yes. It's supposed to have an empty value. I will checkout SpicyPizza/[email protected]. Thanks

@AngelOnFira
Copy link
Member Author

@mannyanebi just so I understand whether it's worth adding some functionality (say a flag that ignores empty values), what's the use case for having an empty field? Do you think this would be a helpful flag?

@mannyanebi
Copy link

mannyanebi commented Feb 15, 2022

I think that it would be a helpful flag for backward compatibility, or handling similar situations that I had earlier on. For the use case, even though it's not recommended to have empty values for env keys, but probably others may want to have that for test purposes or something related to that.

@mannyanebi
Copy link

#42 (comment)

I used SpicyPizza/[email protected] for the action on the project, and it worked out fine. Thanks @AngelOnFira

@nickrupert7
Copy link

Just another voice here, I also ran into this issue where a pipeline on my project broke, since the change was not backwards compatible. Usually you'd consider a breaking change like this to bump the major version (ie SpicyPizza/create-envfile@2), but in the meantime, thank you @AngelOnFira for the quick solution!

@nickrupert7
Copy link

I'd also like to throw it out there that in order to support a forward transition to the new version, it would be nice to have a mechanism for giving undefined env keys a default value.

@AngelOnFira
Copy link
Member Author

I think what I'll do is just push a feature flag that disables the check by default, and you can opt into using it. Seems like the best way to handle this for now. I didn't really think this was enough to warrant a x.0.0 version, but I agree, it definitely isn't great to just have CI break like this (without an easy way to fix it locally)

I'd also like to throw it out there that in order to support a forward transition to the new version, it would be nice to have a mechanism for giving undefined env keys a default value.

I think this solution should remove the need for undefined env keys having a default? If this is still a useful feature, I can extract this into another issue 🙂

@AngelOnFira
Copy link
Member Author

I've updated the tags for v1 and v1.3 to point to the new version, let me know if any other issues come up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants