Fixed bug in the MacOS version check #2819
Conversation
strickvl
added
bug
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughWalkthroughThe updates streamline the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
@coderabbitai review |
Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- src/zenml/cli/utils.py (1 hunks)
- tests/integration/functional/cli/test_utils.py (2 hunks)
Files skipped from review due to trivial changes (1)
- src/zenml/cli/utils.py
Additional context used
Path-based instructions (1)
tests/integration/functional/cli/test_utils.py (1)
Pattern
tests/**/*.py: "Assess the unit test code employing the PyTest testing framework. Confirm that:
- The tests adhere to PyTest's established best practices.
- Test descriptions are sufficiently detailed to clarify the purpose of each test."
Ruff
tests/integration/functional/cli/test_utils.py
145-145: Missing docstring in public function
(D103)
156-156: Missing docstring in public function
(D103)
158-158: Avoid equality comparisons to
False; useif not requires_mac_env_var_warning():for false checksReplace with
not requires_mac_env_var_warning()(E712)
| def test_requires_mac_env_var_warning_non_mac(): | ||
| with patch("sys.platform", "linux"): | ||
| assert requires_mac_env_var_warning() == False |
There was a problem hiding this comment.
Add a docstring and fix the equality comparison.
The function test_requires_mac_env_var_warning_non_mac is missing a docstring. Additionally, avoid equality comparisons to False; use if not requires_mac_env_var_warning(): instead.
def test_requires_mac_env_var_warning_non_mac():
"""Test the requires_mac_env_var_warning function on non-macOS platforms."""
with patch("sys.platform", "linux"):
assert not requires_mac_env_var_warning()Tools
Ruff
156-156: Missing docstring in public function
(D103)
158-158: Avoid equality comparisons to
False; useif not requires_mac_env_var_warning():for false checksReplace with
not requires_mac_env_var_warning()(E712)
| @@ -149,7 +149,7 @@ class KubeflowOrchestratorConfig( | |||
|
|
|||
| kubeflow_hostname: Optional[str] = None | |||
| kubeflow_namespace: str = "kubeflow" | |||
| kubernetes_context: Optional[str] = None # TODO: Potential setting | |||
| kubernetes_context: Optional[str] = None # TODO: Potential setting | |||
There was a problem hiding this comment.
This came in from ruff when I merged in develop.
The version check wasn't handling the difference between OS versions with three vs two numbers. I also added some tests to ensure that it works as expected.
Summary by CodeRabbit
Refactor
Tests