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

feat: add the statusInfo as parameter to the SessionInfo #273

Merged

Conversation

maxl2287
Copy link
Collaborator

What type of PR is this?

Add one of the following kinds:

  • bug
  • correction

What this PR does / why we need it:

Adds the statusInfo as parameter to the SessionInfo

Which issue(s) this PR fixes:

Fixes #267

Copy link

linux-foundation-easycla bot commented Mar 14, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@hdamker
Copy link
Collaborator

hdamker commented Mar 18, 2024

LGTM - but should be merged only after the patch release 0.10.1 is done (and then the documentation need to be changed accordingly again).

Copy link
Collaborator

@RandyLevensalor RandyLevensalor left a comment

Choose a reason for hiding this comment

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

What about using more anonymous urls and public IPs?

code/API_definitions/qod-api.yaml Outdated Show resolved Hide resolved
@hdamker
Copy link
Collaborator

hdamker commented Apr 10, 2024

@maxl2287 would you resolve the merge conflicts? PR #277 changed the SessionInfo structure and the version should be now only "wip" (see my commit).

@maxl2287
Copy link
Collaborator Author

@hdamker, yes I will continue on that next Monday.

…d-statusinfo-to-sessioninfo

# Conflicts:
#	code/API_definitions/qod-api.yaml
@maxl2287
Copy link
Collaborator Author

@hdamker PR is updated to version 0.11.0-wip and conflicts were resolved.

Please take a look.

@maxl2287 maxl2287 requested a review from hdamker April 16, 2024 21:22
Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

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

You have to remove "(the statusInfo parameter is not included in the current version but will be adding to SessionInfo in an upcoming release)" from line 107

@hdamker
Copy link
Collaborator

hdamker commented Apr 17, 2024

You have to remove "(the statusInfo parameter is not included in the current version but will be adding to SessionInfo in an upcoming release)" from line 107

Good catch, reversing the documentation fix for 0.10.1. More precise: the sentence in line 106/107 need to reverse the PR #267, so changing the lines 106/107 back to:

This behavior allows clients which are not receiving notification events but are polling to get the session information with 
the `qosStatus` `UNAVAILABLE` and `statusInfo` `NETWORK_TERMINATED`. Before a client can attempt to create a new QoD session

@maxl2287 maxl2287 requested a review from jlurien April 18, 2024 08:05
Copy link
Collaborator

@hdamker hdamker left a comment

Choose a reason for hiding this comment

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

LGTM

@RandyLevensalor RandyLevensalor merged commit 5ce3d9b into camaraproject:main Apr 19, 2024
3 checks passed
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.

Polling for statusInfo in GET /sessions/{sessionId} not possible
5 participants