Skip to content

Conversation

@Anemy
Copy link
Member

@Anemy Anemy commented Feb 14, 2025

COMPASS-8818

Adds a new telemetry events: Schema Analysis Cancelled.
Updates the existing Schema Exported and Schema Analyzed to have new events.
Updated the way we gather schema metadata for the events to make it so that we only traverse the tree once instead of twice, also makes it so that we unblock the thread occasionally while traversing this tree in case of a very large tree.

We're only tracking the export when folks click the copy button, or click the export button, see discussion in https://mongodb.slack.com/archives/C07JL273J06/p1739389537801429

We only track the optional_field_count and variable_type_count properties for top level fields re https://mongodb.slack.com/archives/C07JL273J06/p1739398926141009

@Anemy Anemy requested a review from paula-stacho February 14, 2025 19:57
@Anemy Anemy added no release notes Fix or feature not for release notes no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) labels Feb 14, 2025
@paula-stacho
Copy link
Collaborator

paula-stacho commented Feb 17, 2025

I wonder if we should distinguish between copy and download?

if (abortSignal?.aborted) {
throw new Error(abortSignal?.reason || new Error('Operation aborted'));
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want this to be an error even though it was an user action? Especially since now it's difficult to get to a re-try, the user basically needs to close&reopen the tab:
Screenshot 2025-02-19 at 10 56 01

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that I didn't notice this sooner

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I'll be changing the error handling in #6733, I can include check for this case and show a warning toast, if we some notification for the user.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good callout, my main reason for changing it to this flow was to align it with the find/documents page:

Screenshot 2025-02-19 at 2 14 57 PM

Folks can still re-try by clicking the Analyze button. That being sad I do agree it's not as discoverable or intuitive as having the big button there. I'm happy going either way here. Would you prefer not having it be an error when someone aborts the operation and return them to the zero state?

Sending you a ping on slack to chat on errors and toasts.

Copy link
Collaborator

@paula-stacho paula-stacho Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toast vs in-tab error TBD. For the question on whether intentional abort is an error - aggregation goes to "no results" state if you cancel.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like both this and the errors in general don't have a clear strategy :/

Copy link
Collaborator

@paula-stacho paula-stacho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! We can figure out the errors with the next PR.

@Anemy Anemy merged commit 1a6ae92 into main Feb 20, 2025
32 of 34 checks passed
@Anemy Anemy deleted the COMPASS-8818 branch February 20, 2025 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no release notes Fix or feature not for release notes no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants