Page MenuHomeSoftware Heritage
Differential D8511

Change targettype in directory entry; add directory entry filter by name
ClosedPublic
Actions

Authored by jayeshv on Sep 20 2022, 3:08 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Commits
rDGQLd445ce72badc: Change value of targetType in DirectoryEntry; add filter by entry name
Summary

targetType is called "dir", "file" and "rev" in DirectoryEntry and is
different from other target types. Change that to "directory", "content"
and "revision"




Add a nameInclude GraphQL level filter for directory entries.
Diff Detail
Repository 
rDGQL GraphQL API 
Lint 
Automatic diff as part of commit; lint not applicable.
 
Unit 
Automatic diff as part of commit; unit tests not applicable.
 
Event Timeline
jayeshv created this revision.Sep 20 2022, 3:08 PM
Herald added a reviewer: Reviewers.  ·  View Herald TranscriptSep 20 2022, 3:08 PM
swh-public-ci added a comment.Sep 20 2022, 3:10 PM
Build is green



Patch application report for D8511 (id=30647)



Rebasing onto ebc6e44534...



Current branch diff-target is up to date.





Changes applied before test



commit 6c44e17d09b79d9c4eaa95f1d2d91765cd60d36d
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 20 15:02:47 2022 +0200

    Change targettype in directory entry; add directory entry filter by name
    
    Target type is called "dir", "file" and "rev" in directory entry it is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL lavel filter for directory entries.



See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/179/ for more details.
Harbormaster completed remote builds in B31626: Diff 30647.Sep 20 2022, 3:10 PM
jayeshv requested review of this revision.Sep 20 2022, 3:10 PM
jayeshv updated this revision to Diff 30650.Sep 20 2022, 4:06 PM
add generic filter for connections
swh-public-ci added a comment.Sep 20 2022, 4:08 PM
Build is green



Patch application report for D8511 (id=30650)



Rebasing onto ebc6e44534...



Current branch diff-target is up to date.





Changes applied before test



commit 5d1c1d87589cf13f16667788c435a1d9f267523b
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 20 15:02:47 2022 +0200

    Change targettype in directory entry; add directory entry filter by name
    
    Target type is called "dir", "file" and "rev" in directory entry it is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL lavel filter for directory entries.



See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/180/ for more details.
Harbormaster completed remote builds in B31629: Diff 30650.Sep 20 2022, 4:08 PM
jayeshv updated this revision to Diff 30652.Sep 20 2022, 4:40 PM
revert generic filters
swh-public-ci added a comment.Sep 20 2022, 4:43 PM
Build is green



Patch application report for D8511 (id=30652)



Rebasing onto ebc6e44534...



Current branch diff-target is up to date.





Changes applied before test



commit e09a9cc59176300dc55d3ecaf1c6b41722e660e0
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 20 15:02:47 2022 +0200

    Change targettype in directory entry; add directory entry filter by name
    
    Target type is called "dir", "file" and "rev" in directory entry it is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL lavel filter for directory entries.



See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/181/ for more details.
Harbormaster completed remote builds in B31631: Diff 30652.Sep 20 2022, 4:43 PM
jayeshv updated this revision to Diff 30660.Sep 21 2022, 9:51 AM
add extra comment; update commit mesasge
jayeshv updated this revision to Diff 30661.Sep 21 2022, 9:52 AM
typo
jayeshv edited the summary of this revision. (Show Details)Sep 21 2022, 9:53 AM
swh-public-ci added a comment.Sep 21 2022, 9:54 AM
Build is green



Patch application report for D8511 (id=30660)



Rebasing onto ebc6e44534...



Current branch diff-target is up to date.





Changes applied before test



commit e87848fc786e2fac2ea76a4a3209321afc346822
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 20 15:02:47 2022 +0200

    Change value of targetType in DirectoryEntry; add filter by entry name
    
    targetType is called "dir", "file" and "rev" in DirectoryEntry and is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL lavel filter for directory entries.



See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/184/ for more details.
Harbormaster completed remote builds in B31638: Diff 30660.Sep 21 2022, 9:54 AM
swh-public-ci added a comment.Sep 21 2022, 9:55 AM
Build is green



Patch application report for D8511 (id=30661)



Rebasing onto ebc6e44534...



Current branch diff-target is up to date.





Changes applied before test



commit c0fccbc38a60baddee3f5dd7d1293b210f585f1d
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 20 15:02:47 2022 +0200

    Change value of targetType in DirectoryEntry; add filter by entry name
    
    targetType is called "dir", "file" and "rev" in DirectoryEntry and is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL level filter for directory entries.



See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/185/ for more details.
Harbormaster completed remote builds in B31639: Diff 30661.Sep 21 2022, 9:55 AM
vlorentz added a subscriber: vlorentz.Sep 21 2022, 11:02 AM
Using .lower() on byte strings of unknown encoding is going to have surprising results.



What is the intended use of making this case-insensitive?
jayeshv added a comment.Sep 21 2022, 11:09 AM


In D8511#220865, @vlorentz wrote:


Using .lower() on byte strings of unknown encoding is going to have surprising results.



What is the intended use of making this case-insensitive?





The intention is to have some kind of a search for contents. Do you think a case sensitive filter is good enough, or should we explicitly handle the error here?
anlambert requested changes to this revision.Sep 21 2022, 2:09 PM
anlambert added a subscriber: anlambert.


In D8511#220866, @jayeshv wrote:




In D8511#220865, @vlorentz wrote:


Using .lower() on byte strings of unknown encoding is going to have surprising results.



What is the intended use of making this case-insensitive?





The intention is to have some kind of a search for contents. Do you think a case sensitive filter is good enough, or should we explicitly handle the error here?





I also think search should be case insensitive here, this is quite a common practice.
swh/graphql/resolvers/directory_entry.py


58
you should call lower before encode here


swh/graphql/tests/functional/test_directory_entry.py


126–149
You should try to make your tests cover more cases and more dynamic by avoiding to hard code test inputs, something like:



@pytest.mark.parametrize("directory", get_directories())
def test_directory_entry_connection_filter_by_name(client, directory):
    storage = server.get_storage()
    for dir_entry in storage.directory_ls(directory.id):
        name_include = dir_entry["name"][:-1].decode()
        query_str = """
      {
        directory(swhid: "%s") {
          swhid
          entries(nameInclude: "%s") {
            nodes {
              targetType
              name {
                text
              }
            }
          }
        }
      }
      """ % (
            directory.swhid(),
            name_include,
        )

        data, _ = utils.get_query_response(client, query_str)

        for entry in data["directory"]["entries"]["nodes"]:
            assert name_include in entry["name"]["text"]
            assert entry["targetType"] == get_target_type(dir_entry["type"])
This revision now requires changes to proceed.Sep 21 2022, 2:09 PM
jayeshv updated this revision to Diff 30667.Sep 21 2022, 2:37 PM
review comments
jayeshv marked 2 inline comments as done.Sep 21 2022, 2:38 PM
swh-public-ci added a comment.Sep 21 2022, 2:39 PM
Build is green



Patch application report for D8511 (id=30667)



Rebasing onto ad9602edd7...



First, rewinding head to replay your work on top of it...
Applying: Change value of targetType in DirectoryEntry; add filter by entry name





Changes applied before test



commit afb80b1563e20b04f6ca087ac30f0292fc225d09
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 20 15:02:47 2022 +0200

    Change value of targetType in DirectoryEntry; add filter by entry name
    
    targetType is called "dir", "file" and "rev" in DirectoryEntry and is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL level filter for directory entries.



See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/188/ for more details.
Harbormaster completed remote builds in B31645: Diff 30667.Sep 21 2022, 2:39 PM
anlambert accepted this revision.Sep 21 2022, 2:57 PM
LGTM
This revision is now accepted and ready to land.Sep 21 2022, 2:57 PM
jayeshv updated this revision to Diff 30669.Sep 21 2022, 3:05 PM
rebase
swh-public-ci added a comment.Sep 21 2022, 3:07 PM
Build is green



Patch application report for D8511 (id=30669)



Rebasing onto ad9602edd7...



Current branch diff-target is up to date.





Changes applied before test



commit d445ce72badc6b6ff51ecb94bf19c51764f25c27
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 20 15:02:47 2022 +0200

    Change value of targetType in DirectoryEntry; add filter by entry name
    
    targetType is called "dir", "file" and "rev" in DirectoryEntry and is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL level filter for directory entries.



See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/189/ for more details.
Harbormaster completed remote builds in B31647: Diff 30669.Sep 21 2022, 3:07 PM
Closed by commit rDGQLd445ce72badc: Change value of targetType in DirectoryEntry; add filter by entry name (authored by jayeshv).  ·  Explain WhySep 21 2022, 3:08 PM
This revision was automatically updated to reflect the committed changes.
jayeshv added a commit: rDGQLd445ce72badc: Change value of targetType in DirectoryEntry; add filter by entry name.
Revision Contents
Changeset List
PathSize
swh/
graphql/
resolvers/
9 lines
6 lines
2 lines
schema/
11 lines
tests/
functional/
38 lines
unit/
resolvers/
5 lines
7 lines
Diff 30670
View Options
swh/graphql/resolvers/directory_entry.py
Loading...
View Options
swh/graphql/resolvers/resolver_factory.py
Loading...
View Options
swh/graphql/resolvers/resolvers.py
Loading...
View Options
swh/graphql/schema/schema.graphql
Loading...
View Options
swh/graphql/tests/functional/test_directory_entry.py
Loading...
View Options
swh/graphql/tests/unit/resolvers/test_resolver_factory.py
Loading...
View Options
swh/graphql/tests/unit/resolvers/test_resolvers.py
Loading...
About · Developer information · Legal