[Draft] [flytekit] Polish - Secrets

[wild-endeavor]

Secrets

This is a series of tickets to improve the flytekit authoring experience. If any changes are not possible to make in a backwards-compatible way, split it out into a separate ticket.

Rename argument

Rename secret_requests argument to secrets for task / eager / dynamic. The fact that it is a request is implied. However, probably add a user-facing note somewhere to make sure they don't accidentally put the actual secret in there.

Union support

Support a single Secret without having to wrap it in a list (i.e. secrets: list[Secret]|Secret|None).

Improve accessing

The current method to retrieve secrets (flytekit.current_context().secrets.get(key="WANDB_API_KEY")) is too verbose. Let's make it something like flytekit.access(key="WANDB_API_KEY").

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

• Yes

Have you read the Code of Conduct?

• Yes

[thomasjpfan]

I agree with renaming secret_requests to secrets and union.

For accessing, I like having secrets in the name somewhere: flytekit.secrets(key="WANDB_API_KEY").

[wild-endeavor]

@cosmicBboy

+1 to flytekit.get_secret("MY_SECRET").

just like this or with the key as @thomasjpfan suggested? If just like this, how do we deal with group? or limit only to secrets with no group?

+1 to remove group and key and just have a single string key (like with union sdk)

this would be backwards incompatible though - is that okay?

[cosmicBboy]

We should also have an explicit environment var name in the secret definition: Secret(..., env_name="MY_ENV_VAR")

[thomasjpfan]

I would continue to have group and key for flytekit. If we want the simple flytekit.get_secret("MY_SECRET") then change the order of key and group:

def get_secret(key: str, group: Optional[str] = None):
    ...

[granthamtaylor]

I would like to also change the order of key and group in flytekit.Secret as well. When one does not use a group, they need to specify the keyword key when defining a Secret instance (flytekit.Secret(key="MY_SECRET")).

This is potentially a breaking change though, but having get_secret have one order (key, group) and Secret having a different order (group, key) feels like a really bad idea.

[thomasjpfan]

get_secret is a new function, so it is not a breaking change. The issue is that it'll be confusing to have different orders between get_secret and Secret.