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

--env-file flag to specify multiple environment variable in a file #560

Closed
kcq opened this issue Aug 17, 2023 · 17 comments
Closed

--env-file flag to specify multiple environment variable in a file #560

kcq opened this issue Aug 17, 2023 · 17 comments

Comments

@kcq
Copy link
Member

kcq commented Aug 17, 2023

The build and profile commands provide a way to specify environment variables (--env flag). Adding many environment variables with the --env flag doesn't scale once you go beyond a few variables.

Need to add a new flag that supports loading multiple environment variables from a file (--env-file).

@reetasingh
Copy link
Contributor

@kcq I am interested in working on this issue. is there any validation we expect to do on the env-file?

@utibeabasi6
Copy link

hey
i would like to work on this if its still open
maybe some guidance to help me get started

@jainhemant163
Copy link

hey i would be interested in contributing this work, can you let me know if this project requirement work is still pending?

@kcq
Copy link
Member Author

kcq commented Aug 21, 2023

@kcq I am interested in working on this issue. is there any validation we expect to do on the env-file?

@reetasingh not much validation is needed loading env var definitions from a file. Something basic should be ok (skipping empty lines and probably checking that the lines you keep looks like an env var definition (e.g., SOME_TEXT=OTHER_TEXT). An example processing a *-file flag: ParseHTTPProbeExecFile (https://github.com/slimtoolkit/slim/blob/master/pkg/app/master/commands/clifvparser.go#L767) you can use as a references.

@kcq
Copy link
Member Author

kcq commented Aug 21, 2023

@utibeabasi6 thank you for volunteering! this is still open. what kind of contribution are you interested in? how well do you know Go? there's a lot of stuff to do in the project. wonder if you'd be open to exploring others things there as well.

@kcq
Copy link
Member Author

kcq commented Aug 21, 2023

hey i would be interested in contributing this work, can you let me know if this project requirement work is still pending?

@jainhemant163 what kind of cycles do you have to contribute and what are your preferences in terms of the type of enhancements / capabilities to focus on? would you be open to more than just this particular enhancement? do you prefer to focus on the CLI related enhancements?

@utibeabasi6
Copy link

@kcq im good with golang, would love to see what i can get my hands on 🙂

@kcq
Copy link
Member Author

kcq commented Aug 23, 2023

@utibeabasi6 great to hear that you are pretty open. The #561 issue can be ok, but you'll need to make enhancements to the sensor, which is usually more complicated. You can give it a try and if it's too much for the first issue we can try something more straight forward. There's a number of issues tagged with good first issue. One of the good issues like that is #373 It can be as much as you can make it enhancing how api-spec-based probing is done. Another potential enhancement to look at is the exec mode for the --exec flag where you can use an array notation for the value (similar to the exec mode for the ENTRYPOINT, CMD or RUN instructions), so the shell in the container image doesn't get included in the minified image (mentioned in this issue: #551 ). There are quite a few non-app related enhancements too (e.g., Github actions, etc) if you are interested in things like that.

@reetasingh
Copy link
Contributor

reetasingh commented Aug 25, 2023

@kcq I am interested in working on this issue. is there any validation we expect to do on the env-file?

@reetasingh not much validation is needed loading env var definitions from a file. Something basic should be ok (skipping empty lines and probably checking that the lines you keep looks like an env var definition (e.g., SOME_TEXT=OTHER_TEXT). An example processing a *-file flag: ParseHTTPProbeExecFile (https://github.com/slimtoolkit/slim/blob/master/pkg/app/master/commands/clifvparser.go#L767) you can use as a references.

Hi @kcq I see that the currently --env flag currently expects a list of string and is being used for container overrides https://github.com/slimtoolkit/slim/blob/master/pkg/app/master/commands/clifvgetter.go#L241
I am assuming it will still be in the form of <env-name1>=<env-value1>, <env-name2>=<env-value2> similar to what we also expect in the .env file.
Also, am i missing any other usage for this flag other than here https://github.com/slimtoolkit/slim/blob/master/pkg/app/master/commands/clifvgetter.go#L241?

@kcq
Copy link
Member Author

kcq commented Sep 7, 2023

@kcq I am interested in working on this issue. is there any validation we expect to do on the env-file?

@reetasingh not much validation is needed loading env var definitions from a file. Something basic should be ok (skipping empty lines and probably checking that the lines you keep looks like an env var definition (e.g., SOME_TEXT=OTHER_TEXT). An example processing a *-file flag: ParseHTTPProbeExecFile (https://github.com/slimtoolkit/slim/blob/master/pkg/app/master/commands/clifvparser.go#L767) you can use as a references.

Hi @kcq I see that the currently --env flag currently expects a list of string and is being used for container overrides https://github.com/slimtoolkit/slim/blob/master/pkg/app/master/commands/clifvgetter.go#L241
I am assuming it will still be in the form of <env-name1>=<env-value1>, <env-name2>=<env-value2> similar to what we also expect in the .env file.
Also, am i missing any other usage for this flag other than here https://github.com/slimtoolkit/slim/blob/master/pkg/app/master/commands/clifvgetter.go#L241?

@reetasingh a minor clarification... the --env-file file data should have new line delimited key=value env var pairs. Comma delimited pair will be brittle because it's hard to know if the comma is the delimiter or if it's a part of the env var value

@reetasingh
Copy link
Contributor

@kcq PTAL #578

@vash-knives
Copy link
Contributor

vash-knives commented Sep 24, 2023

Hi @kcq , I'm interested in contributing to the project, only issue is I'm relatively new to Go but I'm interested in expanding my knowledge of it. Can you point me to some tasks I can jump in and do, relative to my Go knowledge (if you have any) that is. Thanks!

@kcq
Copy link
Member Author

kcq commented Sep 25, 2023

@aclassynerd it's good to start with a quick win. Flag/param validation and UX improvements with the input flags is one of the good categories of improvements. Somewhat related to this task... The --env flag doesn't validate that the value is in the "key=value" format. There's a couple of potential improvements there. Improvement one is to add a format check, so it's "x=y". Improvement two is to print a message to the screen when the --env flag values are ignored when you see they don't have the x=y format, so users know which env flag values are malformed. You'll learn about the commands in Slim and how they are configured. It's also a simple enough project to learn the Go fundamentals and a number of libraries like strings (and its functions like strings.Split and strings.Contains, etc).

@vash-knives
Copy link
Contributor

@kcq , sounds good, ill check those out and maybe this flag/param validation and ux improvements you mentioned. Gonna setup my dev environment now.

@vash-knives
Copy link
Contributor

vash-knives commented Sep 25, 2023

@kcg, diving into the code now, are there tickets / issue numbers for these flag param validation that can be assigned to me? I would like to take a crack at it. I can create them if you don't have any.

@kcq
Copy link
Member Author

kcq commented Sep 27, 2023

@vash-knives here's the ticket for it: #587

@kcq
Copy link
Member Author

kcq commented Oct 20, 2023

Closing... we are all done with this one :)

@kcq kcq closed this as completed Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants