Get a directory entry object with a SWHID and a relative path
Related to T4508
Details
Details
- Reviewers
anlambert - Group Reviewers
Reviewers - Maniphest Tasks
- T4508: GraphQL: Add a directoryEntry entry point
- Commits
- rDGQL1af442fd62fd: Add a directoryEntry entrypoint
Diff Detail
Diff Detail
- Repository
- rDGQL GraphQL API
- Branch
- directory-entry
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 31424 Build 49154: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 49153: arc lint + arc unit
Event Timeline
Comment Actions
Build is green
Patch application report for D8396 (id=30307)
Rebasing onto 467e0b7e28...
Current branch diff-target is up to date.
Changes applied before test
commit 18e15d91977aeb8f6c9bac8bb41cc62d7a0d40ec
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 6 10:47:27 2022 +0200
Add a directoryEntry entrypoint
Get a directory entry object with a SWHID and a relative pathSee https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/150/ for more details.
Comment Actions
Build is green
Patch application report for D8396 (id=30308)
Rebasing onto 467e0b7e28...
Current branch diff-target is up to date.
Changes applied before test
commit 919e9ee5c01a1472e42e7b259ac4f14847e0c431
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 6 10:47:27 2022 +0200
Add a directoryEntry entrypoint
Get a directory entry object with a SWHID and a relative pathSee https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/151/ for more details.
Comment Actions
As this allows nested paths, then the cost calculator needs to take into account the number of / in the path, as each of them requires two joins in the database
Comment Actions
Ok. It will be a bit hard to do in the static cost calculator. But, we can add a higher value for requesting this node irrespective of the path.
The idea of static cost calculator is to reject the query upfront, without even hitting the resolver. That logic is not in master yet. I am still working on some features.
For the dynamic cost calculator (that will also be used for rate limiting in the future), we can have credits depending on the path nesting.
Comment Actions
Build has FAILED
Patch application report for D8396 (id=30358)
Rebasing onto 467e0b7e28...
Current branch diff-target is up to date.
Changes applied before test
commit ed900f8a4dcb8499d9df7be116ca741445ab2b25
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 6 10:47:27 2022 +0200
Add a directoryEntry entrypoint
Get a directory entry object with a SWHID and a relative path
Related to T4508Link to build: https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/154/
See console output for more information: https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/154/console
Comment Actions
Build is green
Patch application report for D8396 (id=30358)
Rebasing onto 467e0b7e28...
Current branch diff-target is up to date.
Changes applied before test
commit ed900f8a4dcb8499d9df7be116ca741445ab2b25
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 6 10:47:27 2022 +0200
Add a directoryEntry entrypoint
Get a directory entry object with a SWHID and a relative path
Related to T4508See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/155/ for more details.
Comment Actions
Build is green
Patch application report for D8396 (id=30397)
Rebasing onto 62920c3258...
Current branch diff-target is up to date.
Changes applied before test
commit 713b2d16173c800968ba33dd729b5022eae8a044
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 6 10:47:27 2022 +0200
Add a directoryEntry entrypoint
Get a directory entry object with a SWHID and a relative path
Related to T4508See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/159/ for more details.
| swh/graphql/tests/functional/test_directory_entry.py | ||
|---|---|---|
| 67–94 | This test could be improved to handle all paths reachable from the root directory. I came up with that code: def test_get_directory_entry_with_nested_path(client, storage): query_str = """ { directoryEntry(swhid: "%s", path: "%s") { name { text } targetType target { ...on Content { swhid } ...on Directory { swhid } } } } """ directory = get_directories_with_nested_path()[0] for entry in storage.directory_ls(directory.id, recursive=True): if entry["type"] == "rev": # TODO: Handle revision target as directory entry (git submodule) continue query = query_str % ( directory.swhid(), entry["name"].decode(), ) data, _ = utils.get_query_response( client, query, ) if entry["type"] == "file": swhid = CoreSWHID( object_type=ObjectType.CONTENT, object_id=entry["sha1_git"] ) elif entry["type"] == "dir": swhid = CoreSWHID( object_type=ObjectType.DIRECTORY, object_id=entry["target"] ) else: swhid = CoreSWHID( object_type=ObjectType.REVISION, object_id=entry["target"] ) assert data["directoryEntry"] == { "name": {"text": entry["name"].decode()}, "target": {"swhid": str(swhid)}, "targetType": entry["type"], } Also directory entry targeting a revision is currently not supported, this should also be fixed. | |
Comment Actions
Build has FAILED
Patch application report for D8396 (id=30404)
Rebasing onto 62920c3258...
Current branch diff-target is up to date.
Changes applied before test
commit f51a840ee7023186b283fe0b4d087794c18dbaf7
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 6 10:47:27 2022 +0200
Add a directoryEntry entrypoint
Get a directory entry object with a SWHID and a relative path
Related to T4508Link to build: https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/160/
See console output for more information: https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/160/console
| swh/graphql/tests/functional/test_directory_entry.py | ||
|---|---|---|
| 67–94 | Thanks. I have added these cases in the previous test. | |
Comment Actions
Build is green
Patch application report for D8396 (id=30405)
Rebasing onto 62920c3258...
Current branch diff-target is up to date.
Changes applied before test
commit cb941a83ccbae1287b54d4657744a6bbe72556fa
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 6 10:47:27 2022 +0200
Add a directoryEntry entrypoint
Get a directory entry object with a SWHID and a relative path
Related to T4508See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/161/ for more details.
| swh/graphql/tests/functional/test_directory_entry.py | ||
|---|---|---|
| 67–94 | Please remove the test_get_directory_entry_with_nested_path test and use parametrized test instead, see below: @pytest.mark.parametrize( "directory", get_directories() + get_directories_with_nested_path() ) def test_get_directory_entry(client, storage, directory): storage = server.get_storage() query_str = """ { directoryEntry(swhid: "%s", path: "%s") { name { text } targetType target { ...on Content { swhid } ...on Directory { swhid } } } } """ for entry in storage.directory_ls(directory.id, recursive=True): if entry["type"] == "rev": # FIXME, Revision is not supported as a directory entry target yet continue query = query_str % ( directory.swhid(), entry["name"].decode(), ) data, _ = utils.get_query_response( client, query, ) swhid = None if entry["type"] == "file" and entry["sha1_git"] is not None: swhid = CoreSWHID( object_type=ObjectType.CONTENT, object_id=entry["sha1_git"] ) elif entry["type"] == "dir": swhid = CoreSWHID( object_type=ObjectType.DIRECTORY, object_id=entry["target"] ) assert data["directoryEntry"] == { "name": {"text": entry["name"].decode()}, "target": {"swhid": str(swhid)} if swhid else None, "targetType": entry["type"], } I also handled the case where a directory entry targets a non archived content as it is a test case in swh-model tests data. | |
| swh/graphql/tests/functional/test_directory_entry.py | ||
|---|---|---|
| 67–94 | There is an issue with parametrized tests. Not everything in the test data are valid db objects but just dummy ids. (They are not None either) | |
Comment Actions
Build is green
Patch application report for D8396 (id=30406)
Rebasing onto 62920c3258...
Current branch diff-target is up to date.
Changes applied before test
commit c6052de67686774c2acff352df9f817ff1edc4c7
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 6 10:47:27 2022 +0200
Add a directoryEntry entrypoint
Get a directory entry object with a SWHID and a relative path
Related to T4508See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/162/ for more details.
| swh/graphql/tests/functional/test_directory_entry.py | ||
|---|---|---|
| 67–94 | Fixed the issue by explicitly adding a condition to check None as id. | |
| swh/graphql/tests/functional/test_directory_entry.py | ||
|---|---|---|
| 67–94 | Tests are still passing with these changes as the unknown object errors are caught by your implementation, (swh) ✔ ~/swh/swh-environment/swh-graphql [arcpatch-D8396_1 L|✚ 1⚑ 3] 11:18 $ make test python3 -m pytest . ================================================================================================================================== test session starts ================================================================================================================================== platform linux -- Python 3.9.2, pytest-7.1.2, pluggy-1.0.0 rootdir: /home/anlambert/swh/swh-environment/swh-graphql, configfile: pytest.ini plugins: redis-2.4.0, asyncio-0.19.0, postgresql-3.1.3, forked-1.4.0, flask-1.2.0, anyio-3.6.1, xdist-2.5.0, cov-3.0.0, dash-2.6.1, requests-mock-1.9.3, django-test-migrations-1.2.0, django-4.5.2, mock-3.8.2, hypothesis-6.54.3, swh.core-2.14.1.dev2+g843a010, swh.journal-1.1.0 asyncio: mode=strict collected 248 items swh/graphql/tests/functional/test_branch_connection.py ............... [ 6%] swh/graphql/tests/functional/test_content.py ............................................. [ 24%] swh/graphql/tests/functional/test_directory.py ....... [ 27%] swh/graphql/tests/functional/test_directory_entry.py ........ [ 30%] swh/graphql/tests/functional/test_origin_connection.py .... [ 31%] swh/graphql/tests/functional/test_origin_node.py ... [ 33%] swh/graphql/tests/functional/test_pagination.py ....... [ 35%] swh/graphql/tests/functional/test_release_node.py .......... [ 39%] swh/graphql/tests/functional/test_revision.py ........ [ 43%] swh/graphql/tests/functional/test_search.py .. [ 43%] swh/graphql/tests/functional/test_snapshot_node.py .... [ 45%] swh/graphql/tests/functional/test_swhid_resolve.py ..................................... [ 60%] swh/graphql/tests/functional/test_visit_node.py ... [ 61%] swh/graphql/tests/unit/errors/test_errors.py ....... [ 64%] swh/graphql/tests/unit/resolvers/test_base_node.py ....... [ 67%] swh/graphql/tests/unit/resolvers/test_resolver_factory.py ...................................... [ 82%] swh/graphql/tests/unit/resolvers/test_resolvers.py ................................ [ 95%] swh/graphql/tests/unit/utils/test_utils.py ........... [100%] ================================================================================================================================== 248 passed in 2.31s ================================================================================================================================== | |
Comment Actions
Build is green
Patch application report for D8396 (id=30407)
Rebasing onto 62920c3258...
Current branch diff-target is up to date.
Changes applied before test
commit 1af442fd62fd1b9d3c5f0b9c8121e3d1f16a2cd6
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Sep 6 10:47:27 2022 +0200
Add a directoryEntry entrypoint
Get a directory entry object with a SWHID and a relative path
Related to T4508See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/163/ for more details.
| swh/graphql/tests/functional/test_directory_entry.py | ||
|---|---|---|
| 67–94 | Got that. the objects are indeed in the DB and the SWHIDs are generated dynamically | |