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.

Sign up for GitHub

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

Jump to bottom

Refactor config management #516

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Refactor config management #516

wants to merge 12 commits into from

Conversation

sweep-ai[bot]
Copy link

@sweep-ai sweep-ai bot commented Nov 10, 2023

PR Feedback (click)

  • 👍 Sweep Did Well
  • 👎 Sweep Needs Improvement

Description

This PR refactors the configuration management in the config.py file to improve code readability and reduce the need for linter pragmas.

The changes include:

  • Replacing the _DEFAULTS dictionary with direct variable definitions.
  • Extracting the logic for loading variables from .env and saving defaults into separate functions.
  • Updating the getenv_fallback, persist_env, and obfuscate functions to work with the new configuration management approach.

Summary of Changes

  • Refactored the config.py file to define configuration variables directly instead of using a dictionary.
  • Extracted the logic for loading variables from .env and saving defaults into separate functions.
  • Updated the getenv_fallback, persist_env, and obfuscate functions to work with the new configuration management approach.

Please review and merge this PR. Thank you!

Fixes #485.

🎉 Latest improvements to Sweep:

💡 To get Sweep to edit this pull request, you can:

  • Comment below, and Sweep can edit the entire PR
  • Comment on a file, Sweep will only modify the commented file
  • Edit the original issue to get Sweep to recreate the PR from scratch

@sweep-ai Sweep AI
Copy link
Author

sweep-ai bot commented Nov 10, 2023

Sandbox Executions

  • Check tests/test_config.py ✗
Sandbox logs for
trunk fmt tests/test_config.py || exit 0 1/2 ✓ 
 ✔ Formatted tests/test_config.py
Re-checking autofixed files...

 ✔ Formatted tests/test_config.py
Re-checking autofixed files...


Checked 1 file
✔ No issues
trunk check --fix --print-failures tests/test_config.py 2/2 ❌ (`1`) 
  ISSUES  
tests/test_config.py:10:0
 10:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
 11:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
 23:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
 27:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
 28:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
 29:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
Checked 1 file
✖ 6 new issues
  • Check tests/test_config.py ✗
Run `tests/test_config.py` through the sandbox.
  • Check tests/test_config.py ✗
Sandbox logs for
trunk fmt tests/test_config.py || exit 0 1/2 ✓ 
 ✔ Formatted tests/test_config.py
Re-checking autofixed files...

 ✔ Formatted tests/test_config.py
Re-checking autofixed files...


Checked 1 file
✔ No issues
trunk check --fix --print-failures tests/test_config.py 2/2 ❌ (`1`) 
  ISSUES  
tests/test_config.py:10:0
 10:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
 11:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
 23:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
 27:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
 28:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
 29:0  low  Use of assert detected. The enclosed code will be removed when compiling to optimised byte       bandit/B101
            code.                                                                                                       
Checked 1 file
✖ 6 new issues

@sweep-ai Sweep AI
Copy link
Author

sweep-ai bot commented Nov 10, 2023

Apply Sweep Rules to your PR?

  • Apply: All new business logic should have corresponding unit tests.
  • Apply: Refactor large functions to be more modular.

@sweep-ai sweep-ai bot added the sweep Assigns Sweep to an issue or pull request. label Nov 10, 2023
@sweep-ai sweep-ai bot mentioned this pull request Nov 10, 2023
Open
5 tasks
abrichr
abrichr reviewed Nov 10, 2023
View reviewed changes
tests/test_config.py Outdated Show resolved Hide resolved
abrichr
abrichr reviewed Nov 10, 2023
View reviewed changes
tests/test_config.py Outdated
config.persist_env("TEST_VAR", "test_value", env_file)
with open(env_file, "r") as f:
lines = f.readlines()
assert "TEST_VAR=test_value\n" in lines
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert "TEST_VAR=test_value\n" in lines
assert "TEST_VAR=test_value" in lines

Copy link
Author

@sweep-ai sweep-ai bot Nov 10, 2023 •
edited

Choose a reason for hiding this comment

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

🚀 Wrote Changes

Done.

abrichr
abrichr reviewed Nov 10, 2023
View reviewed changes
tests/test_config.py Outdated

def test_obfuscate():
assert config.obfuscate("test_value", 0.5, "*") == "*****value"
assert config.obfuscate("another_test_value", 0.3, "#") == "#############t_value"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert config.obfuscate("another_test_value", 0.3, "#") == "#############t_value"
assert config.obfuscate("another_test_value", 0.3, "#") == "#############value"

Copy link
Author

@sweep-ai sweep-ai bot Nov 10, 2023 •
edited

Choose a reason for hiding this comment

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

🚀 Wrote Changes

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abrichr abrichr abrichr left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
sweep Assigns Sweep to an issue or pull request.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor config
1 participant
@abrichr