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

[Flytekit/Pyflyte] Pyflyte register & launchplan commands don't support --envs flag #4092

Closed
Tracked by #4064
PudgyPigeon opened this issue Sep 28, 2023 · 6 comments · Fixed by flyteorg/flytekit#1880
Assignees
Labels
flytekit FlyteKit Python related issue hacktoberfest

Comments

@PudgyPigeon
Copy link
Contributor

PudgyPigeon commented Sep 28, 2023

With pyflyte run commands, you can specify a --envs flag and pass in a JSONified dictionary/object to propagate variables to the container running the workflows/tasks.

Like so:
pyflyte run --envs '{"var1":"hello","var2":"world"}' --image etcetcetc workflowfile

We are requesting an equivalent mode of passing an --envs like dictionary in the context of running pyflyte register and pyflyte launchplan --activate/deactivate commands

Context being:
pyflyte register -p flytesnacks --image imagename:1.0.5 --version v0.0.2a ./workflow.py

And/or
pyflyte launchplan -p flytesnacks -d staging launchplan-name v0.0.1a --activate

@PudgyPigeon
Copy link
Contributor Author

@wild-endeavor

@PudgyPigeon
Copy link
Contributor Author

PudgyPigeon commented Sep 28, 2023

Sorry, I don't mean specifically a Python dictionary but basically a dictionary-like structure/JSON object that you can pass into the container runtime of the pod/task through a pyflyte CLI command - So basically environment variables for the shell/container runtime

@pingsutw pingsutw added flytekit FlyteKit Python related issue hacktoberfest labels Sep 29, 2023
@kumare3
Copy link
Contributor

kumare3 commented Sep 30, 2023

I have updated pyflyte to now support --env X=y --env a=c instead of a dict.

And you want this env car to be set at the launchplan level? Today that is not available

@PudgyPigeon
Copy link
Contributor Author

PudgyPigeon commented Sep 30, 2023

I have updated pyflyte to now support --env X=y --env a=c instead of a dict.

And you want this env car to be set at the launchplan level? Today that is not available

Ah yes, so basically we want to run pyflyte register --envs <jsonified dict> in a way that mirrors/copies the syntax of pyflyte run --envs, if that makes sense.

@MinuraPunchihewa
Copy link

Hey Flyte Team,
Can I take this up?

@PudgyPigeon
Copy link
Contributor Author

PudgyPigeon commented Oct 6, 2023

Hey Flyte Team, Can I take this up?

This PR of ours might help if you want to implement --envs into pyflyte register. Our forks are based off of 1.6.0 so maybe you can do a more updated version - as I think some of the code has been refactored since we forked. Just be sure to credit if you happen to use some of the code in future pushes. :)

https://github.com/PudgyPigeon/flytekit/pull/6/files

One thing to note is that you might run into JSON decoding errors if you pass Cron/whitespace-containing statements as one of the --envs variables. We had to use something like the following to replace the whitespace, so maybe you can take some of the code from here: (We call it cronparser but really it's applicable to any env string that has whitespace)

class CronParser:    
def __init__(self):
    self.replacements = {
        " ": "_"
    }   
def cron_to_string(self, cron: str) -> str:
    return re.sub(r"[\s]", lambda m: self.replacements[m.group()], cron)

def string_to_cron(self, x: str) -> str:
    return re.sub(r"[_]", lambda m: {v: k for k, v in self.replacements.items()}[m.group()], x)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flytekit FlyteKit Python related issue hacktoberfest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants