My only concern with a focus on 3.7.x is that there are immediate problems with 3.5.x that could use attention (and the capabilities of an extracted SimpleWebSocketServer). I suppose it might be possible to work backward from 3.7.x, i.e. make changes and test there, then backport something known to work for 3.7.x to 3.6.x/3.5.x. Not sure if that is practical or not. Or maybe for those earlier versions we have to just keep using the SimpleWebSocketServer the way that we have been in Java only.
Anyway, you laid out the options really nicely and I don't have better suggestions so perhaps gremlin-utils in 3.7.x is the only reasonable approach. On Tue, Oct 25, 2022 at 7:24 PM Cole Greer <[email protected]> wrote: > I believe that proceeding with option 1 is the best choice but it is > not clear how it should be executed. As I see it there are 2 main > questions that need answering. > > Is it better to move the serializers, message classes, etc. into > gremlin-core or is it better to make a new module (perhaps called > something like gremlin-utils) which is shared by gremlin-driver and > gremlin-server? > > I believe that making a new module makes the most sense. Using > gremlin-core for this purpose entered the discussion as a convenient > option due to its place in the dependency tree but the more I consider > it, the more I question if it makes sense to co-opt gremlin-core to > fit this purpose. I suppose package org.apache.tinkerpop.gremlin.util > already exists in gremlin-core so maybe this wouldn't be such a > stretch to have gremlin-core encompass these additional classes. It > really depends on what the community views as the intended purpose and > scope of gremlin-core. If these classes fall within that scope then we > should move these classes into core, otherwise we should proceed with > a new module. > > Which version should these changes be targeted towards? > > The simplest approach is to target this solely to 3.7.x as there it > will be easy to justify minor breaks to users import lines. The > question is what is the value of having these changes in 3.5.x and how > could this be implemented without breaking compatibility? I see 3 > approaches to forcing these changes into 3.5.x: > > a. As previously mentioned we could move the classes to a > different > module but leave the package lines untouched. I've tried this approach > and it seems to work without issue but I agree that it just feels > wrong to have a driver package outside of the driver module. > > b. Leave the original classes untouched in gremlin-driver > (potentially marked as deprecated) and duplicate them to the new > module/core. Original classes could then be removed in 3.7. This could > cause problems if the ClassLoader views these as different and > incompatible types. Also feels like this would be irritating to > maintain in the short term. Any changes to one file would need to be > duplicated as well. > > c. Same as b but instead of leaving the full class behind, > leave > some sort wrapper or subclass in the driver. Shares the same issues as > b except for code duplication. > > None of these strike me as good choices but a. seems the least > problematic. I'm not sure how badly we want to have SimpleSocketServer > or any derivative test servers available to all GLV's in 3.5.x. I > would like to use it if possible for some of the work I'm doing > towards TINKERPOP-2480 but I could always make do without it or delay > support in all GLV's for 3.7. Unless there is a strong desire to make > the test server available in 3.5.x it is probably best to leave it for > 3.7. > > Overall I think it is best to make a new module for these classes and > not to include it until 3.7. I do believe there are still some merits > to bringing improved testing to the GLV's in 3.5.x and 3.6.x but I am > unsure if this is worth dealing with potentially breaking changes and > funky package names. > > Cole Greer > > > On Tue, Oct 25, 2022 at 3:42 AM Stephen Mallette <[email protected]> > wrote: > > > I've considered Option 1 more than once. Perhaps it is just time to do > it. > > I'd be in favor of that, however it would have to be done for 3.7.x as it > > does represent a breaking change in a sense. > > > > Or maybe it isn't a breaking change if you don't change the package > names? > > gremlin-driver already depends on gremlin-core so maybe it doesn't > matter? > > so, for 3.5.x you just move stuff and for 3.7.x I think we'd want to > rename > > the package - "driver" doesn't seem suitable for gremlin-core to me for > > some reason. thoughts? > > > > > > On Mon, Oct 24, 2022 at 12:20 PM Cole Greer <[email protected] > > .invalid> > > wrote: > > > > > Hi Stephen, > > > > > > I was hoping to get some additional feedback regarding the pitch I made > > to > > > make the SimpleSocketServer available to all the GLV's. I agree with > your > > > suggestion to create a new module gremlin-tools/gremlin-socket-server > as > > it > > > seems like a good first step towards some sort of universal test > server. > > > Unfortunately it doesn't immediately resolve any dependency issues. > > > TestWSGremlinInitializer which I would want to include in this new > module > > > depends on several components of gremlin-driver including > RequestMessage, > > > ResponseMessage, and some of the serializers. This is precisely the > > reason > > > that gremlin-server has a dependence on the driver as well and creates > a > > > cyclic dependency problem between the driver and the new test server > > > module. After discussions with the BitQuill team we see 3 options: > > > > > > 1. Refactor gremlin-driver and extract all classes which are needed by > > the > > > server into gremlin-core or some other new module. By moving all of: > > > > > > tinkerpop.gremlin.driver.message.* > > > tinkerpop.gremlin.driver.ser.* > > > tinkerpop.gremlin.driver.MessageSerializer > > > tinkerpop.gremlin.driver.Tokens > > > > > > into a new package, the gremlin-tools/gremlin-socket-server will have > > > access to all of the classes it needs without a dependency on > > > gremlin-driver, resolving the cyclic dependency issue. Furthermore this > > > change would allow us to reduce gremlin-server's dependency on > > > gremlin-driver to a test scope which I believe is an improvement. > > > Pros: Most robust and long term solution. Allows the extraction of > > > SimpleSocketServer from gremlin-driver without losing and functionality > > or > > > ability for future extension. Allows all GLV's to depend on the test > > server > > > without a dependence on gremlin-driver. Provides a good foundation to > > build > > > a universal test server for all the GLV's. > > > Cons: Represents a huge structural change to gremlin-driver. Would > > need > > > a lot of consideration in how to handle this move to minimize user > > impact. > > > 2. Create a docker image from the simple-socket-server as it currently > > > exists as part of the java driver. Publish this image to dockerhub, and > > > configure GLV’s to run tests against this image. > > > Pros: Allows the socket server to continue with all its current > > > features and be accessible to all GLV’s without creating a direct > > > dependency on the java driver. No significant changes required to java > > > driver. > > > Cons: Having a published test image represents a barrier to > changing > > > and adding new functionality or testing behaviours to the > socket-server. > > > > > > 3. Extract Simple-Socket-Server to a new module and remove all > > dependencies > > > on the java driver. Dependence on serializers and ResponseMessage > classes > > > could be stripped out in favour of hardcoded response strings. As it > > stands > > > right now the socket server is only used to test for a small handful of > > > scenarios and it would be fairly straightforward to have a few > hardcoded > > > responses to select from. > > > Pros: Completely removes dependency on java driver without any > large > > or > > > breaking changes. > > > Cons: Limits the ability to grow and extend the current socket > > server. > > > Not a useful step towards the universal test server which Stephen has > > been > > > talking about. > > > > > > Of all of these my preference is to go with number 1. I believe it > would > > be > > > a good improvement to the structure of tinkerpop, but I'm not sure how > > best > > > to handle the migration. 2 and 3 both represent shorter term solutions > to > > > get some use of SimpleSocketServer in the GLV's but don't make the same > > > progress towards standardizing the testing between all the GLV's. I > > suppose > > > a 4th option is to shelve all of this for now and proceed with > > maintaining > > > independent connection tests for all the GLV's but my preference would > be > > > to try and improve on this where possible. > > > > > > Thanks, > > > > > > > > > *Cole Greer* > > > > > > On Mon, Oct 17, 2022 at 5:03 AM Stephen Mallette <[email protected] > > > > > wrote: > > > > > > > If you think it's helpful, I'd say that's fine. Are you suggesting it > > be > > > > moved from gremlin-driver/src/test to gremlin-server/src/main? maybe > it > > > > would go best in a new /server/util/test package? > > > > > > > > The only problem with moving it is that you end up with a circular > > > > relationship. gremlin-server depends on gremlin-driver so if you move > > it > > > > the gremlin-driver tests that depend on it won't work anymore. It's a > > bit > > > > weird but maybe you keep it in gremlin-driver and just move it to > > > > gremlin-driver/.../util/test? > > > > > > > > Or I guess the driver tests could move to gremlin-server along with > the > > > > SimpleSocketServer related classes? that's a bit weird too though. > > > > > > > > Or maybe a new module is on order? create > > > > gremlin-tools/gremlin-socket-server? i think you'd avoid dependency > > > issues > > > > there between gremlin-driver and gremlin-server and the GLVs. Maybe > > this > > > is > > > > the best way as it would allow the independent development of a > really > > > > robust test server without muddying existing packages with test > > > components. > > > > > > > > Sorry, I just sorta rambled my way to that last point which seems > like > > it > > > > could be the best one. > > > > > > > > > > > > > > > > > > > > On Fri, Oct 14, 2022 at 4:27 PM Cole Greer <[email protected] > > > > .invalid> > > > > wrote: > > > > > > > > > Hi everyone, > > > > > > > > > > I have been working on TINKERPOP-2480 according to Ken's recent > > > > > proposal ( > > > > https://lists.apache.org/thread/x8lt6mk04r7h0nsdy9q3pz1pbo1t28tj > > > > > ). > > > > > I have found that the SimpleSocketServer combined with > > > > > TestWSGremlinInitializer in Gremlin-Driver to be a very useful > > > > > resource for creating tests which require custom response behaviour > > > > > from the server. From what I have seen this is something which the > > > > > other GLV's are currently lacking. I believe having such a test > > server > > > > > accessible to the other GLV's would be a useful step towards > > > > > standardizing the testing between all the GLV's and bringing more > > > > > behavioural tests to the GLV's. In order to do this I would propose > > > > > moving the SimpleSocketServer from the tinkerpop.gremlin.driver > > > > > package to tinkerpop.gremlin.server and running it in the existing > > > > > gremlin-server-test docker container on a new port. > > > > > > > > > > I would greatly appreciate any feedback on this idea, whether you > > find > > > > > it to be useful or have any concerns. > > > > > > > > > > Thanks, > > > > > > > > > > Cole > > > > > > > > > > > > > > >
