[Bug] SecretsManager get function supports env_var pararmeters. #3201

0yukali0 · 2025-03-18T17:04:29Z

Tracking issue

Why are the changes needed?

In flytekit secretsManager, it will check whether the secret exist in an env var "" or in a file "/etc/secrets//".
In flytekit secret, create a env "MY_SECRET" when setting "env_var=MY_SECRET" parameters.
SecretManager won't find it with naming rule with "".

What changes were proposed in this pull request?

Adding env_var pararmeters in secreateManager get function.
Initially, secretManager search the secret with following orders.

"_" env
"/etc/secrets//" file
In this PR, its order will be
"_" env
2 . env_var
"/etc/secrets//" file

How was this patch tested?

make test
os.environ["LOCAL_ENV_VAR"] = "value" assert sec.get(group="group", key="key2", env_var="LOCAL_ENV_VAR") == "value" assert sec.get(key="key", env_var="LOCAL_ENV_VAR") == "value" assert sec.get(env_var="LOCAL_ENV_VAR") == "value"

Setup process

Screenshots

Check all the applicable boxes

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

Related PRs

Docs link

Summary by Bito

This PR fixes a bug in SecretsManager by adjusting secret lookup order and adding an optional 'env_var' parameter. It includes renaming and reordering variables in the context_manager module with updated type annotations. The changes enhance reliability and clarity of secret management in flytekit.

Unit tests added: True

Estimated effort to review (1-5, lower is better): 1

welcome · 2025-03-18T17:04:33Z

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
Sign off your commits (Reference: DCO Guide).

flyte-bot · 2025-03-18T17:04:48Z

Code Review Agent Run Status

Limitations and other issues: ❌ Failure - The AI Code Review Agent skipped reviewing this change because it is configured to exclude certain pull requests based on the source/target branch or the pull request status. You can change the settings here, or contact the agent instance creator at [email protected].

flyte-bot · 2025-03-23T16:04:03Z

Code Review Agent Run Status

Limitations and other issues: ❌ Failure - The AI Code Review Agent skipped reviewing this change because it is configured to exclude certain pull requests based on the source/target branch or the pull request status. You can change the settings here, or contact the agent instance creator at [email protected].

flyte-bot · 2025-03-23T18:44:03Z

Code Review Agent Run #2389d5

Actionable Suggestions - 1

flytekit/core/context_manager.py - 1
- Possible undefined variable reference · Line 433-433

Review Details

Files reviewed - 2 · Commit Range: 9503d33..9503d33
- flytekit/core/context_manager.py
- tests/flytekit/unit/core/test_context_manager.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by

flyte-bot · 2025-03-23T18:47:45Z

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted
Bug Fix - SecretsManager Bug Fixes	- `context_manager.py` - Refactored SecretsManager by replacing env_var with default_env_var for proper secret lookup and updated type hints.
Testing - SecretsManager Unit Tests Update	- `test_context_manager.py` - Added unit tests to verify the new env_var parameter and secret retrieval order.

flyte-bot · 2025-03-23T18:47:47Z

flytekit/core/context_manager.py

        if os.path.exists(fpath):
            with open(fpath, encode_mode) as f:
                return f.read().strip()
+        env_vars = ",".join([k for k in filter(None, (group_env_var, env_var))])


Possible undefined variable reference

The variable group_env_var might be undefined when reaching line 433. If none of the environment prefixes in the loop produces a valid group_env_var (e.g., when the loop is not entered or when env_prefixes is empty), then group_env_var will be undefined when constructing the env_vars string. Consider initializing group_env_var to None at the beginning of the function.

Code suggestion

Check the AI-generated fix before applying

@@ -401,0 +402 @@ + group_env_var: Optional[str] = None,

Code Review Run #2389d5

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

Yes, avoid them

flyte-bot · 2025-03-24T01:14:04Z

Code Review Agent Run Status

Limitations and other issues: ❌ Failure - The AI Code Review Agent skipped reviewing this change because it is configured to exclude certain pull requests based on the source/target branch or the pull request status. You can change the settings here, or contact the agent instance creator at [email protected].

flyte-bot · 2025-03-24T16:34:02Z

Code Review Agent Run #98d23d

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: 9503d33..e74db52
- flytekit/core/context_manager.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by

0yukali0 · 2025-03-25T09:54:00Z

/review

flyte-bot · 2025-03-25T09:54:10Z

Code Review Agent Run #f74fd8

Actionable Suggestions - 1

flytekit/core/context_manager.py - 1
- Potential undefined variable in error message · Line 435-435

Review Details

Files reviewed - 2 · Commit Range: 9503d33..e74db52
- flytekit/core/context_manager.py
- tests/flytekit/unit/core/test_context_manager.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by

flyte-bot · 2025-03-25T09:58:15Z

flytekit/core/context_manager.py

        raise ValueError(
            f"Please make sure to add secret_requests=[Secret(group={group}, key={key})] in @task. Unable to find secret for key {key} in group {group} "
-            f"in Env Var:{env_var} and FilePath: {fpath}"
+            f"in Env Var:{env_var}, {group_env_var} and FilePath: {fpath}"


Potential undefined variable in error message

The error message now includes both env_var and group_env_var, but group_env_var might be undefined if the code path doesn't go through the loop at lines 415-422. This could potentially lead to a reference error.

Code suggestion

Check the AI-generated fix before applying

Suggested change

f"in Env Var:{env_var}, {group_env_var} and FilePath: {fpath}"

f"in Env Var:{env_var}{', ' + group_env_var if group_env_var is not None else ''} and FilePath: {fpath}"

Code Review Run #f74fd8

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

Yes, avoid them

Signed-off-by: yuteng <[email protected]>

0yukali0 · 2025-03-26T15:29:18Z

/review

flyte-bot · 2025-03-26T15:29:29Z

Code Review Agent Run #ba9449

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: 33820f9..33820f9
- flytekit/core/context_manager.py
- tests/flytekit/unit/core/test_context_manager.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by

flyte-bot · 2025-03-26T16:34:06Z

Code Review Agent Run #21673e

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: 33820f9..33820f9
- flytekit/core/context_manager.py
- tests/flytekit/unit/core/test_context_manager.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by

0yukali0 changed the title ~~[Bug] SecretsManager get function supports env_var pararmeters.~~ [WIP][Bug] SecretsManager get function supports env_var pararmeters. Mar 20, 2025

0yukali0 force-pushed the flytekit-6302 branch from 4d76b8e to 9503d33 Compare March 23, 2025 14:59

0yukali0 changed the title ~~[WIP][Bug] SecretsManager get function supports env_var pararmeters.~~ [Bug] SecretsManager get function supports env_var pararmeters. Mar 23, 2025

0yukali0 marked this pull request as ready for review March 23, 2025 17:40

0yukali0 requested review from Future-Outlier, cosmicBboy, eapolinario, kumare3, pingsutw, samhita-alla, thomasjpfan and wild-endeavor as code owners March 23, 2025 17:40

flyte-bot reviewed Mar 23, 2025

View reviewed changes

0yukali0 marked this pull request as draft March 23, 2025 23:45

0yukali0 marked this pull request as ready for review March 24, 2025 15:26

flyte-bot reviewed Mar 25, 2025

View reviewed changes

0yukali0 closed this Mar 26, 2025

0yukali0 force-pushed the flytekit-6302 branch from e74db52 to 5503ee5 Compare March 26, 2025 15:27

adding custom env var

33820f9

Signed-off-by: yuteng <[email protected]>

0yukali0 reopened this Mar 26, 2025

	f"in Env Var:{env_var}, {group_env_var} and FilePath: {fpath}"
	f"in Env Var:{env_var}{', ' + group_env_var if group_env_var is not None else ''} and FilePath: {fpath}"

[Bug] SecretsManager get function supports env_var pararmeters. #3201

Are you sure you want to change the base?

[Bug] SecretsManager get function supports env_var pararmeters. #3201

Uh oh!

Conversation

0yukali0 commented Mar 18, 2025 • edited by flyte-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tracking issue

Why are the changes needed?

What changes were proposed in this pull request?

How was this patch tested?

Setup process

Screenshots

Check all the applicable boxes

Related PRs

Docs link

Summary by Bito

Uh oh!

welcome bot commented Mar 18, 2025

Uh oh!

flyte-bot commented Mar 18, 2025

Code Review Agent Run Status

Uh oh!

flyte-bot commented Mar 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run Status

Uh oh!

flyte-bot commented Mar 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #2389d5

Bito Usage Guide

Uh oh!

flyte-bot commented Mar 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelist by Bito

Uh oh!

flyte-bot Mar 23, 2025

Choose a reason for hiding this comment

Uh oh!

flyte-bot commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run Status

Uh oh!

flyte-bot commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #98d23d

Bito Usage Guide

Uh oh!

0yukali0 commented Mar 25, 2025

Uh oh!

flyte-bot commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #f74fd8

Bito Usage Guide

Uh oh!

flyte-bot Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

0yukali0 commented Mar 26, 2025

Uh oh!

flyte-bot commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #ba9449

Bito Usage Guide

Uh oh!

flyte-bot commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #21673e

Bito Usage Guide

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

0yukali0 commented Mar 18, 2025 •

edited by flyte-bot

Loading

flyte-bot commented Mar 23, 2025 •

edited

Loading

flyte-bot commented Mar 23, 2025 •

edited

Loading

flyte-bot commented Mar 23, 2025 •

edited

Loading

flyte-bot commented Mar 24, 2025 •

edited

Loading

flyte-bot commented Mar 24, 2025 •

edited

Loading

flyte-bot commented Mar 25, 2025 •

edited

Loading

flyte-bot commented Mar 26, 2025 •

edited

Loading

flyte-bot commented Mar 26, 2025 •

edited

Loading