You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@tinkerpop.apache.org by vrkrishn <gi...@git.apache.org> on 2016/09/28 06:24:51 UTC

[GitHub] tinkerpop pull request #440: TINKERPOP-1044: Gremlin server REST endpoint - ...

GitHub user vrkrishn opened a pull request:

    https://github.com/apache/tinkerpop/pull/440

    TINKERPOP-1044: Gremlin server REST endpoint - Add Exception Class and Message in Response

    When using Gremlin-Server Client over REST endpoint and the gremlin query results in a groovy script Exception/Error, will output the Throwable class name as well as the Throwable Message in the message body of the HTTP Response

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/vrkrishn/tinkerpop TINKERPOP-1044

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/tinkerpop/pull/440.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #440
    
----
commit de9bbda5e44d5f4a8a7c024894027d1dccd6e2c5
Author: Vivek Krishnan <vi...@microsoft.com>
Date:   2016-09-28T06:21:42Z

    TINKERPOP-1044: When using Gremlin-Server Client over REST endpoint and the gremlin query results in a groovy script Exception/Error, will output the Throwable class name as well as the Throwable Message in the message body of the HTTP Response

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...

Posted by spmallette <gi...@git.apache.org>.

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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...

Posted by vrkrishn <gi...@git.apache.org>.

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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...

Posted by vrkrishn <gi...@git.apache.org>.

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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...

Posted by spmallette <gi...@git.apache.org>.

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 course....I 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...

Posted by spmallette <gi...@git.apache.org>.

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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #440: TINKERPOP-1044: Gremlin server REST endpoint - Add Exc...

Posted by spmallette <gi...@git.apache.org>.

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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---