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

[Access] Unify subscription id with client message id #6847

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

illia-malachyn
Copy link
Contributor

@illia-malachyn illia-malachyn commented Jan 3, 2025

From commit message:

From now on, we use only 1 id in request/response messages. This id is called subscription_id.

A client may provide subscription_id in subscribe request. If client does not provide it, we generate it ourselves.

Clients that use browsers or other async environments may use subscription_id to correlate response messages with the request ones.

subscription_id is used in all messages related to subscription.

I also removed success field from a response. We include subscription_id field in a resposne in case of OK response.
In case of error response, we include error field.

Closes #6845

@illia-malachyn illia-malachyn self-assigned this Jan 3, 2025
From now on, we use only 1 id in request/response
messages. This id is called `subscription_id`.

A client may provide `subscription_id` in `subscribe` request.
If client does not provide it, we generate it ourselves.

Clients that use browsers or other async environemnts
may use `subscription_id` to correlate response messages
with the request ones.

`subscription_id` is used in all messages related to subscription.

I also remove `success` field from response. We include `subscription_id`
field in a resposne in case of OK response.
In case of error response, we include `error` field.
@illia-malachyn illia-malachyn force-pushed the illia-malachyn/6845-unify-subscription-and-message-id branch from 8d1e427 to e5a91d7 Compare January 3, 2025 13:08
@illia-malachyn illia-malachyn changed the title Illia malachyn/6845 unify subscription and message [Access] Unify subscription id with client message id Jan 3, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jan 3, 2025

Codecov Report

Attention: Patch coverage is 55.55556% with 20 lines in your changes missing coverage. Please review.

Project coverage is 41.18%. Comparing base (72adf9e) to head (e5a91d7).

Files with missing lines Patch % Lines
engine/access/rest/websockets/controller.go 54.54% 16 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6847      +/-   ##
==========================================
- Coverage   41.18%   41.18%   -0.01%     
==========================================
  Files        2109     2109              
  Lines      185660   185661       +1     
==========================================
- Hits        76460    76457       -3     
- Misses     102788   102789       +1     
- Partials     6412     6415       +3     
Flag Coverage Δ
unittests 41.18% <55.55%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

[Access] Unify subscription and client IDs
2 participants