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

Introduce GqlStatusObject support as notifications to ResultSummary #1194

Merged
merged 43 commits into from
Jun 20, 2024

Conversation

bigmontz
Copy link
Contributor

@bigmontz bigmontz commented May 16, 2024

⚠️ This API is released as preview.

Introduces ResultSummary.gqlStatusObjects as preview feature.

GqlStatusObject is a GQL compliant Notification object and status of query execution, this new object includes gqlStatus and statusDescription.

ResultSummary.gqlStatusObjects always contains at least 1 status representing the Success, No Data or Omitted Result. When discarding records in a RxSession while connected to a non-GQL-aware server, the driver might not be able to tell apart Success and No Data.

The GqlStatusObjects will be presented in the following order:

  • A “no data” (02xxx) has precedence over a warning;
  • A warning (01xxx) has precedence over a success.
  • A success (00xxx) has precedence over anything informational (03xxx)

Migrating from Notification

Most of the properties present in the Notification are present in the new object, except code and title.
The GqlStatusObject.gqlStatus is equivalent of code in the Notification, but with values compliant with GQL.
The properties GqlStatusObject.classification and GqlStatusObject.rawClassification are equivalent to Notification.category and Notification.rawCategory. The name change is needed because category has a different meaning in GQL.

Configuration

Filtering gqlStatusObjects can be done using in the same way as filtering notifications. However, non-notification status can not be filtered (such as Success or No Data).
The property disabledClassifications was added to NotificationFilter for being named consistently with GQL. This property is equivalent to disabledCategories and it be can used interchangeably. However, they can not be used at the same time. An exception will be raised in this scenario.

⚠️ This API is released as preview.

The bolt protocol 5.5 should support a slightly different way of
configuring notifications filtering.
This new configuration change the name of categories to classification
to be aligned with the definitions on Gql standard.

This changes doesn't includes add the protocol to handshake and the
proper configuration on the driver side.
@bigmontz bigmontz changed the title Introduces ´GqlStatusObject support as notifications to ResultSummary` Introduce ´GqlStatusObject support as notifications to ResultSummary` May 16, 2024
@bigmontz bigmontz changed the title Introduce ´GqlStatusObject support as notifications to ResultSummary` Introduce GqlStatusObject support as notifications to ResultSummary May 17, 2024
@bigmontz bigmontz marked this pull request as ready for review May 31, 2024 09:57
bigmontz and others added 2 commits June 4, 2024 11:30
Co-authored-by: Stephen Cathcart <stephen.m.cathcart@gmail.com>
Copy link
Contributor

@StephenCathcart StephenCathcart left a comment

Choose a reason for hiding this comment

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

Couple of general questions but other than that it seems to meet the ADR spec well 😄 👍

@robsdedude
Copy link
Member

PR description is not up-to-date.

ResultSummary.gqlStatusObjects always contains at least 1 status representing the Success, No Data or Omitted Result. This status will be always the first one.

I thought this isn't the case.

When discarding records while connected to a non-GQL aware server, the driver might not be able to tell apart Success and No Data.

This only applies to the reactive driver, doesn't it? Might be worth being explicit here.

All following status are notifications like warnings about problematic queries or other valuable information that can be presented in a client.

See first point.

packages/bolt-connection/src/bolt/request-message.js Outdated Show resolved Hide resolved
packages/core/test/notification.test.ts Outdated Show resolved Hide resolved
packages/core/test/notification.test.ts Outdated Show resolved Hide resolved
packages/core/test/notification.test.ts Outdated Show resolved Hide resolved
packages/core/test/notification.test.ts Outdated Show resolved Hide resolved
packages/core/test/notification.test.ts Outdated Show resolved Hide resolved
Copy link
Member

@robsdedude robsdedude left a comment

Choose a reason for hiding this comment

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

🔔

packages/bolt-connection/src/bolt/request-message.js Outdated Show resolved Hide resolved
packages/core/src/notification.ts Outdated Show resolved Hide resolved
packages/core/src/result-summary.ts Outdated Show resolved Hide resolved
bigmontz and others added 2 commits June 14, 2024 12:37
Co-authored-by: Robsdedude <dev@rouvenbauer.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants