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

[FlyteCTL Feature] Add support for passing config via ENV vars #4948

Open
2 tasks done
ddl-ebrown opened this issue Feb 25, 2024 · 4 comments
Open
2 tasks done

[FlyteCTL Feature] Add support for passing config via ENV vars #4948

ddl-ebrown opened this issue Feb 25, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request flytectl Issues related to flytectl -Flytes CLI good first issue Good for newcomers

Comments

@ddl-ebrown
Copy link
Contributor

Describe the feature/command for FlyteCTL

pyflyte accepts values like FLYTE_PLATFORM_URL as an override for the admin endpoint in flytes config file

admin:
  # For GRPC endpoints you might want to use dns:///flyte.myexample.com
  endpoint: dns:///flyte:8181
  insecure: true

flytectl does not provide a similar config option. I thought it might be parsed like FLTYE_ADMIN_ENDPOINT or FLYTECTL_ADMIN_ENDPOINT -- but it doesn't look like cobra / viper is configured to parse environment variables in the flytectl binary.

Provide a possible output or UX example

This is useful for providing feature parity for configuration between the two toolchains.

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@ddl-ebrown ddl-ebrown added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers labels Feb 25, 2024
Copy link

dosubot bot commented Feb 25, 2024

🤖

Hey there @ddl-ebrown! 🚀 Great to see you back bringing valuable insights to the table. How's everything going on your end?

Thank you for your feature request. It is indeed possible to add a feature to FlyteCTL that allows passing configuration via environment variables. This can be achieved by using the os package in Go to read environment variables and then use these values to override the configuration values.

Here is a simple example of how you can read an environment variable in Go:

package main

import (
	"fmt"
	"os"
)

func main() {
	platformURL := os.Getenv("FLYTE_PLATFORM_URL")
	fmt.Println(platformURL)
}

In the context of the provided code, you can modify the GetPFlagSet function to read environment variables and use these values to override the default configuration values. Here is an example of how you can do this for the endpoint configuration:

func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet {
	cmdFlags := pflag.NewFlagSet("Config", pflag.ExitOnError)
	
	endpoint := os.Getenv("FLYTE_PLATFORM_URL")
	if endpoint == "" {
		endpoint = defaultConfig.Endpoint.String()
	}
	
	cmdFlags.String(fmt.Sprintf("%v%v", prefix, "endpoint"), endpoint, "For admin types,  specify where the uri of the service is located.")
	
	// ... rest of the code ...
	
	return cmdFlags
}

In this example, the os.Getenv function is used to read the FLYTE_PLATFORM_URL environment variable. If this environment variable is not set (i.e., it returns an empty string), the default endpoint value is used. Otherwise, the value of the environment variable is used.

This approach is similar to the one used in a previously solved issue in the Flyte repository: [BUG] Platform config fields other than 'endpoint' and 'insecure' cannot be overriden in flyte-cli commands. The issue was addressed by modifying the GetPFlagSet function to read environment variables and use these values to override the default configuration values.

I hope this helps! If you have any further questions or need more clarification, please let me know.

Sources

About Dosu

This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@pingsutw pingsutw added flytectl Issues related to flytectl -Flytes CLI and removed untriaged This issues has not yet been looked at by the Maintainers labels Feb 29, 2024
@eapolinario eapolinario added the good first issue Good for newcomers label Mar 1, 2024
@eapolinario
Copy link
Contributor

@ddl-ebrown , this is a great feature. Please, feel free to contribute. ❤️

Dosu's answers pinpoint exactly in the codebase where this change would take place. I'm happy to review a PR.

@zychen5186
Copy link
Contributor

#self-assign

@dalaoqi
Copy link

dalaoqi commented Nov 11, 2024

#take

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request flytectl Issues related to flytectl -Flytes CLI good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants