-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add ability to explain dagScanNode
attribute(s).
#560
feat: Add ability to explain dagScanNode
attribute(s).
#560
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #560 +/- ##
===========================================
+ Coverage 56.17% 56.33% +0.15%
===========================================
Files 121 121
Lines 14276 14291 +15
===========================================
+ Hits 8020 8051 +31
+ Misses 5557 5542 -15
+ Partials 699 698 -1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got a couple of comments for you, mostly in the tests.
5a2f92d
to
e7a45b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving assuming you address Andy's todo 🙂
e7a45b8
to
00a91d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - thanks for expanding the tests (is a nitpick in there but no worries if left as is)
00a91d3
to
13ae9af
Compare
…rk#560) - Relevant issue: Resolves sourcenetwork#479 - Description: Adds the `spans` attribute for `dagScanNode` to be included in the returned explain graph response. - Request: ``` query @Explain { allCommits (dockey: "test-key", field: "1") { links { cid } } } ``` - Response: ``` { "explain": { "selectTopNode": { "selectNode": { "filter": null, "commitSelectNode": { "dagScanNode": { "spans": []{ { "start": "/test-key/1", "end": "/test-key/2", } } } } } } } } ```
Relevant issue(s)
Resolves #479
Description
Adds the
spans
attribute fordagScanNode
to be included in the returned explain graph response.Example:
Request:
Response:
Tasks
How has this been tested?
Integration tests locally and only CI. Specifically
make test
.Specify the platform(s) on which this was tested: