search

gchq / Gaffer

A large-scale entity and relation database supporting aggregation of properties
Apache License 2.0
1.77k stars 350 forks source link

Gh-3183 Remove gafferpop specifics from operation chains #3184

Closed tb06904 closed 6 months ago

tb06904 commented 6 months ago

Fixes issue with the current Gafferpop library where the resulting operation chain that would be ran on the graph included the element generators specific to Gafferpop. They are now called locally on the results of a chain or before running a chain so that the target graph does not need to be aware of the Gafferpop library. This also avoids any serialisation errors when using the proxy store to enable gremlin on an existing graph.

Other fixes

Tested some of this manually spinning up a Map Store REST and attaching a gremlin server via a proxy store to allow querying.

Related issue

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 91.37931% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 66.71%. Comparing base (669c98b) to head (2d8c079).

Files Patch % Lines
...a/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraph.java 91.37% 5 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #3184 +/- ## ============================================= - Coverage 66.72% 66.71% -0.01% - Complexity 2557 2558 +1 ============================================= Files 907 907 Lines 29060 29067 +7 Branches 3243 3242 -1 ============================================= + Hits 19390 19392 +2 - Misses 8236 8243 +7 + Partials 1434 1432 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

t92549 commented 6 months ago

It's definitely a different approach that will work better for proxy stores but worse for Accumulo stores. This probably just points to a problem in Gaffer where proxy stores fail to execute operations that aren't present on the remote store. I can't remember if there is a way to make them execute locally without using a FederatedStore though. If not, that could perhaps be a separate improvement.