Page MenuHomeSoftware Heritage
Differential D5577

adds a filter by node type as a query argument
ClosedPublic
Actions

Authored by Hakimb on Apr 22 2021, 5:27 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDGRPH85880ae74999: adds a filter by node type as a query argument
Summary

We now have a "return types" query argument on endpoints:
*leaves
*neighbors
*visit/nodes
*random walk.

This allows us to filter the returned nodes by discriminating the nodes returned during the visit by a set of node types.
The argument is a string that takes the form :

"*" or "nodetype" or "nodetype,nodetype...."

Diff Detail

Repository
rDGRPH Compressed graph representation
Branch
node-type-filter
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20985
Build 32569: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 32568: arc lint + arc unit

Event Timeline

Hakimb created this revision.Apr 22 2021, 5:27 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 22 2021, 5:27 PM
swh-public-ci added a comment.Apr 22 2021, 5:29 PM

Build is green

Patch application report for D5577 (id=19933)

Rebasing onto 04a1656c10...

First, rewinding head to replay your work on top of it...
Applying: adds a filter by node type as a query argument
Changes applied before test
commit 65755f09f9197fde2a85af18b5b993914bf6b12a
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Thu Apr 22 17:20:14 2021 +0200

    adds a filter by node type as a query argument

See https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/90/ for more details.

Harbormaster completed remote builds in B20938: Diff 19933.Apr 22 2021, 5:29 PM
Hakimb requested review of this revision.Apr 22 2021, 5:29 PM
vlorentz added a subscriber: vlorentz.Apr 22 2021, 5:52 PM

Looks good overall, but I'm not familiar with this piece of code.

Could you add some more comments in NodesFiltering.java? Two examples would be nice too.

If possible, could you add tests?

Some small style comments below:

java/src/main/java/org/softwareheritage/graph/Entry.java
117–119

What is the difference between t.leavesVisitor(srcNodeId, this::writeNode); and this?

(same question for the other two functions below)

java/src/main/java/org/softwareheritage/graph/NodesFiltering.java
6–10

What about overloading the constructor with a variant with no argument, instead of a magic value *?

25–27

Wouldn't this work?

for (String type : types) {
    restrictedNodesTypes.add(Node.Type.fromStr(type));
}

or even this?

Collections.addAll(restrictedNodesTypes, types)
36–41

Doesn't this return a value *and* modify the argument? this could be confusing to use.

What about creating a new ArrayList and adding matching elements to it?

swh/graph/server/app.py
100–105

no need to create a temporary list, instead we can use a generator expression to pass a generator to any.

109–110

use comments

vlorentz requested changes to this revision.Apr 22 2021, 5:52 PM
This revision now requires changes to proceed.Apr 22 2021, 5:52 PM
vlorentz added inline comments.Apr 22 2021, 5:57 PM
java/src/main/java/org/softwareheritage/graph/NodesFiltering.java
36–41

And I'm not very familiar with Java, but isn't ArrayList backed by a single dynamic array? This means every remove operation needs to copy all elements on the right, so filterByNodeTypes runs in quadratic time.

Hakimb added a comment.Apr 23 2021, 4:17 PM
java/src/main/java/org/softwareheritage/graph/Entry.java
117–119

In Traversal.java, we can see that the functions "leaves" "neighbors" etc... are not used, because the code you show calls directly leavesVisitor with, as "consumer", a function that displays on the output the nodes found during the visit, on the fly.

However, I need to filter the nodes found after the visit, not during, so I used the functions "leaves" "neighbors" etc... which store in an arraylist all the nodes found during the visit, to be able to do my filtering, and I still use the writeNode function to display them at the end.

Hakimb updated this revision to Diff 19984.Apr 23 2021, 6:11 PM

adding comments, tests, and fix code style

swh-public-ci added a comment.Apr 23 2021, 6:12 PM

Build is green

Patch application report for D5577 (id=19984)

Rebasing onto 04a1656c10...

First, rewinding head to replay your work on top of it...
Applying: adds a filter by node type as a query argument
Applying: adds a filter by node type as a query argument
Changes applied before test
commit 025054c31f8cf5c5d680db8e87b870e73361709e
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Fri Apr 23 18:09:25 2021 +0200

    adds a filter by node type as a query argument

commit 073a54a86120c71dd36600dcb7bf889640359014
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Thu Apr 22 17:20:14 2021 +0200

    adds a filter by node type as a query argument

See https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/94/ for more details.

Harbormaster completed remote builds in B20985: Diff 19984.Apr 23 2021, 6:12 PM
Hakimb updated this revision to Diff 19985.Apr 23 2021, 6:25 PM

add comment and examples of queries

swh-public-ci added a comment.Apr 23 2021, 6:26 PM

Build is green

Patch application report for D5577 (id=19985)

Rebasing onto 04a1656c10...

First, rewinding head to replay your work on top of it...
Applying: adds a filter by node type as a query argument
Applying: adds a filter by node type as a query argument
Applying: add comment and examples of queries
Changes applied before test
commit 16a8f167b74b6939cf3fe6e6da51fe3f2c7fd2b0
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Fri Apr 23 18:24:30 2021 +0200

    add comment and examples of queries

commit 05927df6b86aceb50d88c424513a6432a001f8c4
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Fri Apr 23 18:09:25 2021 +0200

    adds a filter by node type as a query argument

commit a7732696e21b9afb296642c5a9612f1c00ac4bcd
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Thu Apr 22 17:20:14 2021 +0200

    adds a filter by node type as a query argument

See https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/95/ for more details.

Harbormaster completed remote builds in B20986: Diff 19985.Apr 23 2021, 6:27 PM
vlorentz added a comment.Apr 26 2021, 9:51 AM
In D5577#141601, @vlorentz wrote:

Could you add some more comments in NodesFiltering.java? Two examples would be nice too.

I meant examples with (executable) code.

Hakimb updated this revision to Diff 20091.Apr 28 2021, 12:45 PM
  • add comments and examples in NodeFiltering class
swh-public-ci added a comment.Apr 28 2021, 12:45 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/100/
See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/100/console

Harbormaster failed remote builds in B21083: Diff 20091!Apr 28 2021, 12:45 PM
Hakimb updated this revision to Diff 20093.Apr 28 2021, 1:52 PM

rebase and add comments and examples in NodeFiltering class

swh-public-ci added a comment.Apr 28 2021, 1:54 PM

Build is green

Patch application report for D5577 (id=20093)

Rebasing onto 6f21897a7e...

Current branch diff-target is up to date.
Changes applied before test
commit 452b503cc26571695f2d9fbbcc8d514529c38168
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Wed Apr 28 13:51:24 2021 +0200

    add a filter by node type as a query parameter

See https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/101/ for more details.

Harbormaster completed remote builds in B21085: Diff 20093.Apr 28 2021, 1:54 PM
vlorentz added a comment.Apr 29 2021, 9:42 AM

Thanks! One last thing: as this code block documents the whole class, could you move it at the beginning of the file?

java/src/main/java/org/softwareheritage/graph/NodesFiltering.java
89
Hakimb updated this revision to Diff 20258.May 4 2021, 4:33 PM
  • fix comment
swh-public-ci added a comment.May 4 2021, 4:33 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/111/
See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/111/console

Harbormaster failed remote builds in B21245: Diff 20258!May 4 2021, 4:33 PM
Hakimb updated this revision to Diff 20259.May 4 2021, 4:39 PM

fix diff

swh-public-ci added a comment.May 4 2021, 4:41 PM

Build is green

Patch application report for D5577 (id=20259)

Rebasing onto 1a8b00f4cd...

First, rewinding head to replay your work on top of it...
Applying: adds a filter by node type as a query argument
Using index info to reconstruct a base tree...
M	java/src/main/java/org/softwareheritage/graph/Entry.java
M	java/src/main/java/org/softwareheritage/graph/Traversal.java
M	swh/graph/backend.py
M	swh/graph/client.py
M	swh/graph/graph.py
M	swh/graph/server/app.py
Falling back to patching base and 3-way merge...
Auto-merging swh/graph/server/app.py
CONFLICT (content): Merge conflict in swh/graph/server/app.py
Auto-merging swh/graph/graph.py
CONFLICT (content): Merge conflict in swh/graph/graph.py
Auto-merging swh/graph/client.py
CONFLICT (content): Merge conflict in swh/graph/client.py
Auto-merging swh/graph/backend.py
CONFLICT (content): Merge conflict in swh/graph/backend.py
Auto-merging java/src/main/java/org/softwareheritage/graph/Traversal.java
CONFLICT (content): Merge conflict in java/src/main/java/org/softwareheritage/graph/Traversal.java
Auto-merging java/src/main/java/org/softwareheritage/graph/Entry.java
CONFLICT (content): Merge conflict in java/src/main/java/org/softwareheritage/graph/Entry.java
Patch failed at 0001 adds a filter by node type as a query argument

Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

Rebase failed (ret=1)!




Could not rebase; Attempt merge onto 1a8b00f4cd...



Already up to date.





Changes applied before test



commit 073881b6f2e7898306f15f79f75e72ab7a58cb31
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue May 4 16:30:46 2021 +0200

    adds a filter by node type as a query argument

commit 2cbb0bb4e20c5538c5426f4642ad54a4d4511673
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Wed Apr 28 12:44:52 2021 +0200

    add comments and examples in NodeFiltering class

commit 2c206f35bce1100d4bc34dabb8e5b0aa54390008
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Fri Apr 23 18:24:30 2021 +0200

    add comment and examples of queries

commit beb9de1def38c04e5408e0a3465d769f8c6b795f
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Fri Apr 23 18:09:25 2021 +0200

    adds a filter by node type as a query argument

commit b788d817c33a42302e7257a2b8cb7afc76c7fa6c
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Thu Apr 22 17:20:14 2021 +0200

    adds a filter by node type as a query argument



See https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/112/ for more details.
Harbormaster completed remote builds in B21246: Diff 20259.May 4 2021, 4:41 PM
vlorentz accepted this revision.May 4 2021, 4:43 PM
This revision is now accepted and ready to land.May 4 2021, 4:43 PM
vlorentz requested changes to this revision.May 4 2021, 5:01 PM
Er, I didn't notice but you have too many commits in the diff now, please squash them
This revision now requires changes to proceed.May 4 2021, 5:01 PM
Hakimb updated this revision to Diff 20262.May 4 2021, 5:03 PM
conflicts merge fixed
swh-public-ci added a comment.May 4 2021, 5:06 PM
Build has FAILED



Patch application report for D5577 (id=20262)



Rebasing onto 1a8b00f4cd...



First, rewinding head to replay your work on top of it...
Applying: adds a filter by node type as a query argument
Using index info to reconstruct a base tree...
M	java/src/main/java/org/softwareheritage/graph/Entry.java
M	java/src/main/java/org/softwareheritage/graph/Traversal.java
M	swh/graph/backend.py
M	swh/graph/client.py
M	swh/graph/graph.py
M	swh/graph/server/app.py
Falling back to patching base and 3-way merge...
Auto-merging swh/graph/server/app.py
CONFLICT (content): Merge conflict in swh/graph/server/app.py
Auto-merging swh/graph/graph.py
CONFLICT (content): Merge conflict in swh/graph/graph.py
Auto-merging swh/graph/client.py
CONFLICT (content): Merge conflict in swh/graph/client.py
Auto-merging swh/graph/backend.py
CONFLICT (content): Merge conflict in swh/graph/backend.py
Auto-merging java/src/main/java/org/softwareheritage/graph/Traversal.java
CONFLICT (content): Merge conflict in java/src/main/java/org/softwareheritage/graph/Traversal.java
Auto-merging java/src/main/java/org/softwareheritage/graph/Entry.java
CONFLICT (content): Merge conflict in java/src/main/java/org/softwareheritage/graph/Entry.java
Patch failed at 0001 adds a filter by node type as a query argument

Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".



Rebase failed (ret=1)!






Could not rebase; Attempt merge onto 1a8b00f4cd...



Already up to date.





Changes applied before test



commit 9316536b9b923873bd3a29311699b5b5d77a5a23
Merge: 1a8b00f 073881b
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue May 4 16:56:22 2021 +0200

    adds a filter by node type as a query argument

commit 073881b6f2e7898306f15f79f75e72ab7a58cb31
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue May 4 16:30:46 2021 +0200

    adds a filter by node type as a query argument

commit 2cbb0bb4e20c5538c5426f4642ad54a4d4511673
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Wed Apr 28 12:44:52 2021 +0200

    add comments and examples in NodeFiltering class

commit 2c206f35bce1100d4bc34dabb8e5b0aa54390008
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Fri Apr 23 18:24:30 2021 +0200

    add comment and examples of queries

commit beb9de1def38c04e5408e0a3465d769f8c6b795f
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Fri Apr 23 18:09:25 2021 +0200

    adds a filter by node type as a query argument

commit b788d817c33a42302e7257a2b8cb7afc76c7fa6c
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Thu Apr 22 17:20:14 2021 +0200

    adds a filter by node type as a query argument



Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/113/

See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/113/console
Harbormaster failed remote builds in B21249: Diff 20262!May 4 2021, 5:06 PM
vlorentz added a comment.EditedMay 4 2021, 5:14 PM
You added another commit. To clean up everything, I suggest that you drop the existing commits and create a new one. Here is how you can do it:



    

  1. Make sure you are on your master branch
    2. 

  2. Removes your commit from the local master branch, so the state of the branch is the same as the main (origin) repo: git reset origin/master. This will keep the local content of the files (as unstaged changes), because you don't pass --hard.
    3. 

  3. Create a new commit from scratch, based on the local content: git add [...] then git commit -m "adds a filter by node type as a query argument"
    4. 

  4. finally, update the diff: arc diff --update D5577 -m "squash commits"
    5. 

Hakimb updated this revision to Diff 20264.May 4 2021, 5:17 PM
squash commits
vlorentz accepted this revision.May 4 2021, 5:18 PM
This revision is now accepted and ready to land.May 4 2021, 5:18 PM
swh-public-ci added a comment.May 4 2021, 5:18 PM
Build has FAILED



Patch application report for D5577 (id=20264)



Rebasing onto 1a8b00f4cd...



Current branch diff-target is up to date.





Changes applied before test



commit 35391f89acfb33d542d3b6182142ce364418af47
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue May 4 17:17:09 2021 +0200

    adds a filter by node type as a query argument



Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/114/

See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/114/console
Harbormaster failed remote builds in B21251: Diff 20264!May 4 2021, 5:18 PM
Hakimb updated this revision to Diff 20265.May 4 2021, 5:31 PM
fix argument in Entry and Traversal + an assert in test_api_client
swh-public-ci added a comment.May 4 2021, 5:34 PM
Build is green



Patch application report for D5577 (id=20265)



Rebasing onto 1a8b00f4cd...



Current branch diff-target is up to date.





Changes applied before test



commit 85880ae749995213856cea9b9afad5a2b59f519c
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue May 4 17:30:10 2021 +0200

    adds a filter by node type as a query argument



See https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/115/ for more details.
Harbormaster completed remote builds in B21252: Diff 20265.May 4 2021, 5:34 PM
Closed by commit rDGRPH85880ae74999: adds a filter by node type as a query argument (authored by Hakimb).  ·  Explain WhyMay 4 2021, 5:38 PM
This revision was automatically updated to reflect the committed changes.
Hakimb added a commit: rDGRPH85880ae74999: adds a filter by node type as a query argument.
vlorentz mentioned this in D5672: Traversal: Fix ignored maxEdges being ignored in overloaded constructor..May 4 2021, 6:16 PM
Revision Contents
Changeset List
PathSizeCoverage (All)Coverage (Touched)
java/
src/
main/
java/
org/
softwareheritage/
graph/
40 lines--
60 lines--
46 lines--
swh/
graph/
12 lines88%
Loading...
42 lines93%
Loading...
9 lines84%
Loading...
server/
33 lines36%
Loading...
tests/
45 lines84%
Loading...
Diff 19984
View Options
java/src/main/java/org/softwareheritage/graph/Entry.java
Loading...
View Options
java/src/main/java/org/softwareheritage/graph/NodesFiltering.java
Loading...
View Options
java/src/main/java/org/softwareheritage/graph/Traversal.java
Loading...
View Options
swh/graph/backend.py
Loading...
View Options
swh/graph/client.py
Loading...
View Options
swh/graph/graph.py
Loading...
View Options
swh/graph/server/app.py
Loading...
View Options
swh/graph/tests/test_api_client.py
Loading...
About · Developer information · Legal