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

Change default fallback for unrecognised REST group text commands #16237

Merged

Conversation

tkountis
Copy link
Contributor

Fixes #15957

Added a new REST group too for CP APIs, and fixed some of the CP URIs.

I was also tempted to throw handle the error differently for each protocol, but at this stage of filtering the incoming request, for unified networking, we don't know whether its HTTP or Memcache. Therefore we rely on the previous way of just closing the connection without responding.

@tkountis tkountis added this to the 4.0 milestone Dec 11, 2019
@tkountis tkountis self-assigned this Dec 11, 2019
@@ -78,8 +78,8 @@
public static final String URI_CP_SUBSYSTEM_BASE_URL = "/hazelcast/rest/cp-subsystem";
public static final String URI_RESET_CP_SUBSYSTEM_URL = URI_CP_SUBSYSTEM_BASE_URL + "/reset";
public static final String URI_CP_GROUPS_URL = URI_CP_SUBSYSTEM_BASE_URL + "/groups";
public static final String URI_CP_SESSIONS_SUFFIX = "/sessions";
public static final String URI_REMOVE_SUFFIX = "/remove";
public static final String URI_CP_SESSIONS_SUFFIX = URI_CP_SUBSYSTEM_BASE_URL + "/sessions";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@metanet was this intended or a mistake? If the former, please let me know so I can revert.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this was intended as it's a suffix that can occur at the end of some URI, for instance see here and here.

@tkountis tkountis force-pushed the fix/4.0/rest-groups-default-to-null branch from 34ca170 to ce43bb7 Compare December 12, 2019 10:00
@mmedenjak
Copy link
Contributor

You can also rebase as I see some URIs have been removed in the meantime.

@tkountis tkountis force-pushed the fix/4.0/rest-groups-default-to-null branch 2 times, most recently from 7337afa to b85ee4b Compare December 13, 2019 12:39
@tkountis tkountis force-pushed the fix/4.0/rest-groups-default-to-null branch from b85ee4b to e9d29ab Compare December 13, 2019 15:18
@mmedenjak mmedenjak merged commit a7dec9d into hazelcast:master Dec 13, 2019
@mmedenjak
Copy link
Contributor

Thank you for the PR @tkountis and the review @kwart ! 😉

mdogan added a commit to mdogan/hazelcast that referenced this pull request Dec 18, 2019
CP subsystem was using `CLUSTER_WRITE_ group before
but a new `CP` group is added by hazelcast#16237.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Throw exception when REST URI does not match any pattern
3 participants