-
Notifications
You must be signed in to change notification settings - Fork 59
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
Add ability to set envvars from the cli #179
base: main
Are you sure you want to change the base?
Conversation
Came across an issue #138 that also adds the ability to load from a file. I think this could be added with this PR potentially. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for your contribution! This looks fairly good. Just a couple of adjustments and it will be ready to land.
} | ||
|
||
const opts = { | ||
envVars: await parsePairs(rawArgs._).catch((e) => error(e)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will want to introduce other operations related to envs: list, delete, export, import from .env file, etc. I think it would be preferable if we already plan for this by having a set
subcommand (ie deployctl env set key=value key2=value2
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good to know, I was unsure at first due to the only API I could see was to set (i.e making a PATCH request to the api) but this makes sense.
also with #185 looking to also add option for file would it make sense for us to use the same subcommand? some thing like env set [key=value]
and env set --env-file
? or better think about merging those later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was unsure at first due to the only API I could see was to set (i.e making a PATCH request to the api)
The PATCH is also used to remove envs, by setting null
as value.
also with #185 looking to also add option for file would it make sense for us to use the same subcommand? some thing like env set [key=value] and env set --env-file? or better think about merging those later?
Yes, we definitely want that feature, and I think it makes sense to have it within the env
subcommand. However, I wonder if we wouldn't prefer a separate env subcommand, like deployctl env set key=value [keyn=valuen]
and deployctl env import [--env-file env-file]
. In any case, let's coordinate with @drollinger.
Adding also @kwhinnery to the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be happy with this, will check to see if @kwhinnery has any thoughts before implementing my changes as I have time to try and complete this soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adoublef I would go ahead and implement deployctl env set key=value
and deployctl env import [--env-file env-file]
.
Have been away from this for a while, wondering if this is still something I can continue or shall close. thanks |
I created a simple subcommand for setting environment variables from the CLI tool. There are a fair different ways that variables can be added but I've chosen one for now. If setting variables from a file would be of interest or setting via a prompt I am happy to alter