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

Pull secrets from environment when running locally #2800

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thomasjpfan
Copy link
Member

@thomasjpfan thomasjpfan commented Oct 10, 2024

Why are the changes needed?

When running locally, it is not natural to have a _FSEC_ prefix on your secrets.

What changes were proposed in this pull request?

With this PR, the _FSPEC_ prefix is not used anymore for local execution.

How was this patch tested?

I added unit tests to this PR to check the new behavior and ran this test:

from flytekit import task, Secret
from flytekit import current_context


@task(
    secret_requests=[Secret(key="apikey", group="mygroup")],
    container_image="localhost:30000/flytekit:dev",
)
def check_secret() -> str:
    ctx = current_context()
    secret = ctx.secrets.get(key="apikey", group="mygroup")
    return secret
export MYGROUP_APIKEY=my-secret
pyflyte run --remote main.py check_secret

Setup process

Screenshots

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Signed-off-by: Thomas J. Fan <[email protected]>
env_prefixes = [self._env_prefix]

# During local execution check for the key without a prefix
is_local_execution = os.getenv("FLYTE_INTERNAL_EXECUTION_ID") is None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we convert this to utility function as i think it could be useful in other places to know if you are in local mode or not . But don't we have --remote flag that we can use to determine the same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants