gchq / gaffer-tools

gaffer-tools is deprecated. Use https://github.com/gchq/gafferpy instead
Apache License 2.0
50 stars 29 forks source link

Gh 1018: Refactor fishbowl to use jinja2 #1034

Closed t92549 closed 1 year ago

t92549 commented 1 year ago

Related Issue

t92549 commented 1 year ago

This diff shows a lot of changes to the generated files simply because they are unsorted each time they are generated (due to an issue with the api: https://github.com/gchq/Gaffer/issues/2775) I have fixed this issue in this PR in fishbowl by sorting the results before generation: https://github.com/gchq/gaffer-tools/blob/d3f6f7a3ac8b208a08a491bbc6017ecfe462cee4/python-shell/src/fishbowl/fishbowl.py#L113 For an actual diff of how they have changed, see https://github.com/gchq/gaffer-tools/pull/1034/commits/d3f6f7a3ac8b208a08a491bbc6017ecfe462cee4

codecov-commenter commented 1 year ago

Codecov Report

Merging #1034 (94640cf) into v2-alpha (f640ae3) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             v2-alpha    #1034   +/-   ##
===========================================
  Coverage       45.15%   45.15%           
  Complexity        101      101           
===========================================
  Files              25       25           
  Lines             897      897           
  Branches           73       73           
===========================================
  Hits              405      405           
  Misses            462      462           
  Partials           30       30           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2af6914...94640cf. Read the comment docs.

GCHQDeveloper314 commented 1 year ago

This diff shows a lot of changes to the generated files simply because they are unsorted each time they are generated (due to an issue with the api: gchq/Gaffer#2775) I have fixed this issue in this PR in fishbowl by sorting the results before generation

Does this mean that issue in Gaffer is no longer required because the issue can be resolved by sorting in the rest-api instead of in Gaffer? Looks like that issue can be closed once this PR is merged.

t92549 commented 1 year ago

This diff shows a lot of changes to the generated files simply because they are unsorted each time they are generated (due to an issue with the api: gchq/Gaffer#2775) I have fixed this issue in this PR in fishbowl by sorting the results before generation

Does this mean that issue in Gaffer is no longer required because the issue can be resolved by sorting in the rest-api instead of in Gaffer? Looks like that issue can be closed once this PR is merged.

So it could be closed because, at least for this use case, that is solved. However, I still think it would be nice if the api itself was consistent and didn't need to be sorted client side every time