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

Fixed trailing / behind resource in registration API. As described in… #1

Closed
wants to merge 1 commit into from

Conversation

breuhan
Copy link

@breuhan breuhan commented Feb 22, 2018

… RegistrationAPI "/resource"

@garethsb
Copy link
Contributor

Thanks for the PR!

I think the proposed change to nmos/registration_api.cpp (used in nmos-cpp-registry) is incorrect as things stand. The current implementation follows the example in the specification at:
https://github.com/AMWA-TV/nmos-discovery-registration/blob/v1.2.x/examples/registrationapi-v1.1-base-get-200.json

However I agree the first change to nmos/node_registration.cpp (used in nmos-cpp-node) for the POST to /resource may be a good idea. This has been discussed previously, resulting in @andrewbonney drafting the following guidance in AMWA-TV/is-04#15:

A server implementing the APIs MUST support PUT/PATCH/POST/DELETE/OPTIONS (ie. write related) operations on the non-trailing slash version of each resource. The implementation MAY additionally support the same methods on the trailing slash version but this is not strictly required.

Unfortunately, I don’t think that has yet been merged into the spec. Thanks for reminding me of this! It’s possible that when it is, the response for the API “base” response should be changed also.

As for the other change to nmos/node_registration.cpp for the DELETE, unless I’m mistaken the path to be appended at that point doesn’t contain a leading slash, so the trailing slash on "/resource/" is required?

@garethsb
Copy link
Contributor

Resolved by commit 7cd46cd.

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.

2 participants