feat(data-modeling): add an option to disable auto infer when generating a new diagram COMPASS-9777 #7328
Conversation
…ing a new diagram
gribnoysup
added
the
feature flagged
| // @ts-expect-error Element is accepted, but not typed correctly | ||
| description={ | ||
| <> | ||
| Using a special algorithm, Compass will try to automatically |
There was a problem hiding this comment.
Minor nits
"Using a special algo" can be removed. This might just cause people to ask for details on that algo
| Using a special algorithm, Compass will try to automatically | ||
| discover relationships in selected collections. This operation | ||
| will run multiple find requests against{' '} | ||
| <strong>indexed fields</strong> of the collections and might |
There was a problem hiding this comment.
"and will take additional time per collection being analyzed"
There was a problem hiding this comment.
and maybe this is what should be bolded, since this is where we expect there to be customer impact
|
|
||
| enableAutomaticRelationshipInference: { | ||
| stage: 'development', | ||
| stage: 'released', |
There was a problem hiding this comment.
do we remove FF's once they've been fully released?
There was a problem hiding this comment.
We generally keep them around as no-ops since there's no harm in doing so and Compass would otherwise reject usage of the feature flag. Not a huge deal either way, though.
There was a problem hiding this comment.
Sooo... we should be cleaning them up over time, but practically we always forget to schedule this work, so a lot of very old flags are hanging out in the codebase (tell me if this sounds familiar @johnjackweir 😄) and as Anna correctly noticed there's not really much harm in that and so very little pressure for cleaning up
The main reason we have a dedicated released state is to be able to easy roll back in the first few weeks of the feature being available by just flipping the value instead of, for example, reverting the patches that would remove the flag from the code completely. But after this "trial" period we should be cleaning those up, in theory
| onCollectionsSelect: (colls: string[]) => void; | ||
| onAutomaticallyInferRelationshipsToggle: (newVal: boolean) => void; | ||
| }) { | ||
| const showAutoInferOption = usePreference( |
There was a problem hiding this comment.
Basic Compass FF question, can this be removed now that the FF is enabled?
There was a problem hiding this comment.
Yes – but see Sergey's answer below for why we may want to keep it for a bit.
There was a problem hiding this comment.
I don't think this one really need to stay for that long, I really wanted it only for the initial algo merging while there was no form checkbox, I'll clean this up in a week or so.
|
Updated the wording, please take another look! |
3 participants
This patch adds a checkbox that allows to opt-out of automatic relationship discovery logic when generating a new diagram. Should be easier to review with no whitespace changes