-
Notifications
You must be signed in to change notification settings - Fork 212
feat: update atlas cluster states COMPASS-8228 #6884
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
Conversation
This reverts commit 2a14569.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good, I like the hook and how we're managing this. Left a couple questions, mostly from me not being too familiar with the different cluster states.
@@ -48,7 +48,7 @@ export interface AtlasClusterMetadata { | |||
| 'UPDATING' | |||
| 'PAUSED' | |||
| 'IDLE' | |||
| 'REPARING' | |||
| 'REPAIRING' | |||
| 'DELETING' | |||
| 'DELETED'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a related comment above, is SUCCESSor WORKING a state we want to have here? Where's the source of truth for these?
It's not this right?
https://github.com/10gen/mms/blob/8f63b6c6e79f50a310a662260131f2f05818cdd8/server/src/main/com/xgen/cloud/nds/project/_public/model/ClusterDescription.java#L4681
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const storeRef = useRef(useStore()); | ||
const [ref] = useState(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be a cleaner way to write this?
const store = useStore();
const getConnectable = useCallback((connectionId: string) => {
// ...
}, [store]);
Fine as is too I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, would that have the same outcome?
packages/compass-connections/src/stores/connections-store-redux.ts
Outdated
Show resolved
Hide resolved
@@ -1399,7 +1427,9 @@ function getDescriptionForNonRetryableError(error: Error): string { | |||
// to the generic error description. | |||
const reason = error.message.match(/code: \d+, reason: (.*)$/)?.[1]; | |||
return reason && reason.length > 0 | |||
? reason | |||
? reason.endsWith('.') | |||
? reason.slice(0, -1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL negative numbers work with slice
Err once unit tests are green and we should probably add some tests of our own. |
https://jira.mongodb.org/browse/COMPASS-8228
Description
Screen.Recording.2025-04-30.at.5.30.46.PM.mov
Screen.Recording.2025-04-30.at.5.33.21.PM.mov
Screen.Recording.2025-04-30.at.5.44.29.PM.mov
Checklist
New tests and/or benchmarks are included
Documentation is changed or added
If this change updates the UI, screenshots/videos are added and a design review is requested
I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)
Motivation and Context
Open Questions
Dependents
Types of changes