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

Revert "Merge pull request #578 from armosec/temp_remove_tst" #579

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

afek854
Copy link
Contributor

@afek854 afek854 commented Jan 9, 2025

User description

This reverts commit ccc56f0, reversing changes made to 1fc780c.


PR Type

Bug fix, Tests


Description

  • Reverts a previous commit that removed registry tests.

  • Restores the start method functionality in registry_connectors.py.

  • Ensures registry connection checks are re-enabled.


Changes walkthrough 📝

Relevant files
Bug fix
registry_connectors.py
Restore `start` method functionality in registry tests     

tests_scripts/registry/registry_connectors.py

  • Removed the return statement from the start method.
  • Restored functionality for registry connection checks.
  • +0/-1     

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

    This reverts commit ccc56f0, reversing
    changes made to 1fc780c.
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Return Value

    The start() method no longer returns any value after removing the early return statement. Consider adding an appropriate return value at the end of the method.

    def start(self):
        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')

    Copy link

    github-actions bot commented Jan 9, 2025

    Failed to generate code suggestions for PR

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Restore method's return value to maintain consistent function signature and error handling

    Add a return statement at the end of the start method to indicate success/failure
    status, as the method previously had a return value that was removed.

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

     def start(self):
         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]: 8

    Why: The suggestion correctly identifies that removing the return statement could break the method's contract and error handling pattern. Restoring the return value is important for maintaining consistent API behavior and proper error handling throughout the codebase.

    8

    @afek854 afek854 merged commit f32b4b5 into master Jan 9, 2025
    2 checks passed
    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.

    2 participants