[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/440 This was a pretty simple change - merge this via CTR --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/440 Ok - I'll look to just merge this as-is then will add in the change to the other channelizers. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user vrkrishn commented on the issue: https://github.com/apache/tinkerpop/pull/440 Would it be possible to push this change then and add the other functionality as a different ticket? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/440 We intend to freeze the code base for release of 3.2.3 (and 3.1.5) in a couple of days. If you'd like to see this merged in time for 3.2.3, we'd need to have your changes pretty soon for review. No pressure, of courseI just wanted to inform you of our release schedule in case you really needed this change in an official packaged release. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user vrkrishn commented on the issue: https://github.com/apache/tinkerpop/pull/440 Yeah I can take care of the AbstractEvalOpProcessor --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/440 Glad that's working. Did you still intend to make the fix for the other channelizers in `AbstractEvalOpProcessor`? As for titan, i guess you would build this branch of tinkerpop with the work ongoing here: https://github.com/thinkaurelius/titan/pull/1312 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user vrkrishn commented on the issue: https://github.com/apache/tinkerpop/pull/440 Luckily I was able to spin up a Linux VM and run the integration tests Tests run: 31, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 162.807 sec - in org.apache.tinkerpop.gremlin.server.GremlinServerIntegrateTest Results : Tests run: 163, Failures: 0, Errors: 0, Skipped: 13 Also, I am currently using titan 1 which depends on gremlin-server 3.1 i think, what would be the best way to adapt this change to my current distro of titan? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/440 Sorry for the trouble on running the tests. It's not terribly friendly to Windows as we don't have any core devs who use that OS. Hopefully you can figure out how to get the tests working. Maybe they won't run well with maven, but might run from an IDE like Intellij? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user vrkrishn commented on the issue: https://github.com/apache/tinkerpop/pull/440 I'm having trouble running the tests on my Windows Box (the tests fail on different modules so I don't think that this change caused the failures). I also see that the CI build passed which should indicate that the unit tests that are not running probably fail to run due to Windows-specific shenanigans. I'll update this once I can successfully run tests on my box --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/440 First of all, thanks for taking the time to do this. [TINKERPOP-1044](https://issues.apache.org/jira/browse/TINKERPOP-1044) was less about REST and more about the Gremlin Server protocol but as it turns out the issue is the same in REST as it is over there. > there should be some common response between all channelizer methods that is defined in only one location There's no such place to do that. There isn't any shared exception processing logic between REST and the Gremlin Server protocol. I think you would add that logic here: https://github.com/apache/tinkerpop/blob/850e159b8dc883ac3995c8d775b8f09b7a64b440/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java#L260-L266 as i look at it now the error messaging logic is a bit weird there to begin with (i.e. if we don't have an "error message" we just use the class simple name?? - no idea why that is like that). Note that there are a number of integration tests that may fail if with these changes you are making as they rely on the asserting error messages in some cases. If you haven't done so already please be sure to run those tests: ```text mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false ``` Also, please add an entry to the CHANGELOG that describes your fix: https://github.com/apache/tinkerpop/blob/850e159b8dc883ac3995c8d775b8f09b7a64b440/CHANGELOG.asciidoc#tinkerpop-323-release-date-not-officially-released-yet --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user rjbriody commented on the issue: https://github.com/apache/tinkerpop/pull/440 > there should be some common response between all channelizer methods that is defined in only one location Could be. I'm not familiar enough with the channelizers to know for sure. I'm mostly just selfishly interested in getting the exception type when using the java driver, so this inconsistency jumped out at me. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user vrkrishn commented on the issue: https://github.com/apache/tinkerpop/pull/440 I added this functionality to the REST endpoint because I am using the HttpChannelizer, I can add the functionality to the other channelizers manually but I think you are getting at that there should be some common response between all channelizer methods that is defined in only one location --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...
Github user rjbriody commented on the issue: https://github.com/apache/tinkerpop/pull/440 Nice work. Looks good for the REST endpoint. I'm wondering - what about other endpoints such as websocket - shouldn't they be consistent? Also, I would expect the same information to be available when using a gremlin driver. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
