You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "jinmeiliao (GitHub)" <gi...@apache.org> on 2019/01/10 22:54:09 UTC

[GitHub] [geode] jinmeiliao opened pull request #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

…gmt/v2/regions

Co-authored-by: Jens Deppe <jd...@pivotal.io>
Co-authored-by: Peter Tran <pt...@pivotal.io>

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?

- [ ] Is your initial contribution a single, squashed commit?

- [ ] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.


[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pdxrunner commented on pull request #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "pdxrunner (GitHub)" <gi...@apache.org>.
Why would we have have a valid `reponseBody` if not authorized?

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] aditya87 commented on pull request #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "aditya87 (GitHub)" <gi...@apache.org>.
I think it might be weird to give CREATED status when we haven't actually created anything?

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on issue #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "jinmeiliao (GitHub)" <gi...@apache.org>.
I am starting to think to need to redo this in a entirely different way. Will hold off merging.

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on issue #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "jinmeiliao (GitHub)" <gi...@apache.org>.
@aditya87 @Petahhh 

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pdxrunner commented on pull request #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "pdxrunner (GitHub)" <gi...@apache.org>.
Add a check for the expected `responseBody`

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on pull request #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "jinmeiliao (GitHub)" <gi...@apache.org>.
this is just a dummy implementation, it's returnning CREATED so that later on we don't have to change the test expected status

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-amurmann commented on pull request #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "pivotal-amurmann (GitHub)" <gi...@apache.org>.
same question about the name as in GeodeAdminRestClient

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-amurmann commented on issue #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "pivotal-amurmann (GitHub)" <gi...@apache.org>.
It would be awesome if the commit message could adhere to format guidelines and even better have a little bit more of a description of what it's doing. Are there restrictions on what can be posted? Can I make any kind of region with this now? Any interesting design decisions that went into this?

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-amurmann commented on pull request #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "pivotal-amurmann (GitHub)" <gi...@apache.org>.
Might something like `basePath` be a more descriptive name?

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on pull request #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "jinmeiliao (GitHub)" <gi...@apache.org>.
`context` means a webapp's name in here. Essentially we have two webapps hosted in our jetty server, `geode` and `geode-mgmt`, one for dev rest, one for admin rest. The name here is the `context`.

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao closed pull request #3066: GEODE-6174: Added rest controller to process post request to /geode-m…

Posted by "jinmeiliao (GitHub)" <gi...@apache.org>.
[ pull request closed by jinmeiliao ]

[ Full content available at: https://github.com/apache/geode/pull/3066 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org