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

return success status in RegistryChecker start method #578

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

kooomix
Copy link
Contributor

@kooomix kooomix commented Jan 9, 2025

PR Type

Bug fix


Description

  • Added a success status return in RegistryChecker.start method.

  • Ensures consistent return values for the start method.


Changes walkthrough 📝

Relevant files
Bug fix
registry_connectors.py
Add success status return to `start` method                           

tests_scripts/registry/registry_connectors.py

  • Added a return statement with statics.SUCCESS and an empty string.
  • Ensures the start method explicitly returns a success status.
  • +1/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @kooomix kooomix merged commit ccc56f0 into master Jan 9, 2025
    2 checks passed
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Unreachable Code

    The early return statement at the start of the method makes all subsequent code unreachable, including critical setup and installation steps

    return statics.SUCCESS, ""

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Fix early return statement that prevents execution of critical method functionality

    Move the return statement to the end of the method. The current placement causes the
    method to return before executing any of its core functionality, making the rest of
    the code unreachable.

    tests_scripts/registry/registry_connectors.py [39-44]

     def start(self):
    -    return statics.SUCCESS, ""
         Logger.logger.info('Stage 1: Install kubescape with helm-chart')
         self.cluster, _ = self.setup(apply_services=False)
    +    self.install_kubescape()
    +    Logger.logger.info('Stage 2: Check registries connection')
    +    return statics.SUCCESS, ""
    • Apply this suggestion
    Suggestion importance[1-10]: 10

    Why: The suggestion identifies a critical bug where an early return statement would prevent the execution of essential functionality including kubescape installation and registry connection checks. This would completely break the intended behavior of the method.

    10

    Copy link

    github-actions bot commented Jan 9, 2025

    Failed to generate code suggestions for PR

    afek854 added a commit that referenced this pull request Jan 9, 2025
    This reverts commit ccc56f0, reversing
    changes made to 1fc780c.
    afek854 added a commit that referenced this pull request Jan 9, 2025
    Revert "Merge pull request #578 from armosec/temp_remove_tst"
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant