fix: connectivity check buttons: update state field behaviour && state indicator enhancement. #15561
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @FaheemOnHub, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've addressed a critical issue where the connectivity check buttons in the UI were not displaying the correct state because the underlying GraphQL subscription was not being properly initialized or updated. My changes ensure that the subscription is always active and responsive to configuration changes, providing accurate and real-time connectivity status in the UI.
Highlights
- Fix GraphQL Subscription Initialization: I've added an explicit call to
mesheryControllerSubscription.initSubscription()to ensure that the GraphQL subscription for controller state is always properly started wheninitSubscriptionsis invoked. - Improve Controller State Subscription Logic: I've refactored the
useEffecthook that monitorsk8sConfigchanges. Now, it intelligently checks ifmesheryControllerSubscriptionexists. If it does, the subscription is updated; otherwise,initSubscriptionsis called to re-initialize it, ensuring the connectivity state is always accurate. - Code Style Consistency: I've wrapped a single-line
returnstatement ininitSubscriptionswith curly braces for better code style consistency.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where GraphQL subscriptions were not being initialized, leading to connectivity status not updating correctly. The changes correctly call initSubscription and also handle initializing the subscription within the useEffect hook when k8sConfig changes. My review includes a suggestion to fix the dependency array of a useEffect to prevent potential bugs from stale closures, which is a critical aspect of working with React hooks.
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [k8sConfig, capabilitiesRegistry]); |
There was a problem hiding this comment.
This useEffect hook has missing dependencies. The effect's body uses state and initSubscriptions, but they are not included in the dependency array. This can lead to the effect running with stale values, which can cause subtle bugs that are hard to track down.
The eslint-disable-next-line react-hooks/exhaustive-deps comment should be removed, and the dependency array should be updated to correctly include all values from the outer scope that are used within the effect.
}, [k8sConfig, capabilitiesRegistry, state, initSubscriptions]);|
|
867fe96 to
060ee2f
Compare
|
Commit SHA: END-TO-END TESTS
📦 Test Result Summary
⌛ Duration: 9 minutes and 28 seconds Overall Result: 👎 Some tests failed. [Show/Hide] Test Result Details
|
Notes for Reviewers
1.All the connectivity check buttons,lights , status and versions are working perfectly now. The issue was that the gql subscription was never called , resulting in always null.
2.Enhanced notifications and intermediate-state(amber) , to make user aware of the progress.
480388950-78162f2b-500e-412d-9d79-ec1fb9cace83.mov
Signed commits