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

[BUG] Flytectl create execution doesn't respect targetProject or targetDomain #4628

Open
2 tasks done
katrogan opened this issue Dec 20, 2023 · 3 comments
Open
2 tasks done
Labels
bug Something isn't working waiting for reporter Used for when we need input from the bug reporter

Comments

@katrogan
Copy link
Contributor

katrogan commented Dec 20, 2023

Describe the bug

When I specify a distinct targetProject and targetDomain in the execFile passed to flytectl create execution

e.g.

targetDomain: "production"
targetProject: "flytetester"
workflow: basics.hello_world.my_wf

but reference a workflow registered in flytesnacks-development like so:

$ flytectl create execution --execFile execution_spec.yaml -p flytesnacks -d development --version TRRaOBOdkLw5HhMeM5Y2RQ==
execution identifier project:"flytesnacks" domain:"development" name:"fa5aec886c8f2495da11" 

the launched execution ends up in flytesnacks-development rather than flytetester-production

Expected behavior

Launching an execution in a different project and/or domain should be possible via flytectl

Additional context to reproduce

See description

Screenshots

No response

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

  • Yes

Have you read the Code of Conduct?

  • Yes
@katrogan katrogan added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Dec 20, 2023
Copy link

dosubot bot commented Dec 20, 2023

🤖

Hello @katrogan,

Nice to meet you! I'm Dosu, an AI assistant here to help you with your Flyte inquiries. While we're waiting for a human maintainer, I'm here to assist you with solving bugs, answering your questions, and helping you become an even better contributor to our project. Let's work together to get your issue sorted out!

The behavior you're experiencing is due to the way the flytectl create execution command is currently designed. The targetProject and targetDomain specified in the execFile are not being respected because the AdminExecutionCreateRequest struct, which is used to launch an execution, only includes fields for Project, Domain, Name, Spec, and Inputs. There are no fields for targetProject or targetDomain in this struct.

Here is the relevant part of the code:

type AdminExecutionCreateRequest struct {
	Project string `json:"project,omitempty"`
	Domain string `json:"domain,omitempty"`
	Name string `json:"name,omitempty"`
	Spec *AdminExecutionSpec `json:"spec,omitempty"`
	Inputs *CoreLiteralMap `json:"inputs,omitempty"`
}

As you can see, there are no targetProject or targetDomain fields in this struct. The Project and Domain fields in the AdminExecutionCreateRequest struct are populated with the project and domain specified in the command line when the execution is launched. If you want to launch the execution in a different project or domain, you would need to specify those in the command line when launching the execution.

You can find this code in the flyteidl/gen/pb-go/flyteidl/service/flyteadmin/model_admin_execution_create_request.go file.

I hope this clarifies the current behavior. If you have any further questions or need additional clarification, please let me know.

Sources


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.

@eapolinario
Copy link
Contributor

A few things here:

  1. IMO cli flags should take precedence over what's defined in the execution spec.
  2. flytectl is being too strict here and requiring both project and domain to be passed as flags.

The combination of 1. and 2. creates this confusion where in order to specify a different project/domain we can only use the cli flags.

Instead, we should relax the need for project/domain in the case of create execution if the exec spec defines them. We should clarify precedence in the help of create execution.

wdyt, @katrogan ?

@eapolinario eapolinario added waiting for reporter Used for when we need input from the bug reporter and removed untriaged This issues has not yet been looked at by the Maintainers labels Dec 21, 2023
@katrogan
Copy link
Contributor Author

katrogan commented Dec 21, 2023

That sounds great! I was a little confused at first by whether target referred to the reference workflow/task in the execFile or the desired execution time value and improving the clarity around how we parse the flags would be helpful

another thing that threw me off is that the execFile expects a task or workflow, although conventionally we launch executions by referencing a task or launch plan. Can file an issue to track that as well if you think it's worthwhile @eapolinario

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working waiting for reporter Used for when we need input from the bug reporter
Projects
None yet
Development

No branches or pull requests

2 participants