Page MenuHomeSoftware Heritage

jayeshv (Jayesh)
User

Projects

User Details

User Since
Mar 17 2021, 3:56 PM (79 w, 4 d)

Recent Activity

Thu, Sep 22

jayeshv updated the task description for T4367: GraphQL: Pagination fixes to comply with the Relay spec.
Thu, Sep 22, 6:31 PM · GraphQL API
jayeshv renamed T4367: GraphQL: Pagination fixes to comply with the Relay spec from GraphQL: Fix issue in pagination 'after' argument to GraphQL: Pagination fixes to comply with the Relay spec.
Thu, Sep 22, 6:31 PM · GraphQL API
jayeshv retitled D8518: Refactor node and connection resolver bindings from Refactor node resolver bindings to Refactor node and connection resolver bindings.
Thu, Sep 22, 3:36 PM
jayeshv updated the diff for D8518: Refactor node and connection resolver bindings.

commit message change

Thu, Sep 22, 3:36 PM
jayeshv updated the diff for D8518: Refactor node and connection resolver bindings.

connection resolver refactoring

Thu, Sep 22, 3:34 PM
jayeshv requested review of D8522: Add nameExcludePrefix filter to snapshot branch.
Thu, Sep 22, 2:56 PM
jayeshv abandoned D8334: [WIP] Add a dynamic query cost calculator.
Thu, Sep 22, 10:40 AM
jayeshv updated the diff for D8518: Refactor node and connection resolver bindings.

doc updates

Thu, Sep 22, 9:52 AM
jayeshv retitled D8518: Refactor node and connection resolver bindings from [WIP] Refactor resolver bindings to Refactor node resolver bindings.
Thu, Sep 22, 9:19 AM
jayeshv updated the diff for D8518: Refactor node and connection resolver bindings.

commit message update

Thu, Sep 22, 9:18 AM
jayeshv updated the diff for D8518: Refactor node and connection resolver bindings.

type hints

Thu, Sep 22, 9:17 AM

Wed, Sep 21

jayeshv retitled D8518: Refactor node and connection resolver bindings from [WIP] Add filters for latest visit status to [WIP] Refactor resolver bindings.
Wed, Sep 21, 5:59 PM
jayeshv updated the diff for D8518: Refactor node and connection resolver bindings.

refactoring

Wed, Sep 21, 5:42 PM
jayeshv updated the diff for D8518: Refactor node and connection resolver bindings.

missing functions

Wed, Sep 21, 4:31 PM
jayeshv updated the diff for D8518: Refactor node and connection resolver bindings.

rebase

Wed, Sep 21, 4:22 PM
jayeshv requested review of D8518: Refactor node and connection resolver bindings.
Wed, Sep 21, 4:14 PM
jayeshv closed D8515: Add filters for latest visit status.
Wed, Sep 21, 3:33 PM
jayeshv committed rDGQL32d5564f9724: Add filters for latest visit status (authored by jayeshv).
Add filters for latest visit status
Wed, Sep 21, 3:33 PM
jayeshv updated the diff for D8515: Add filters for latest visit status.

rebase

Wed, Sep 21, 3:30 PM
jayeshv added inline comments to D8515: Add filters for latest visit status.
Wed, Sep 21, 3:22 PM
jayeshv updated the diff for D8515: Add filters for latest visit status.

review comments

Wed, Sep 21, 3:21 PM
jayeshv closed D8511: Change targettype in directory entry; add directory entry filter by name.
Wed, Sep 21, 3:08 PM
jayeshv committed rDGQLd445ce72badc: Change value of targetType in DirectoryEntry; add filter by entry name (authored by jayeshv).
Change value of targetType in DirectoryEntry; add filter by entry name
Wed, Sep 21, 3:08 PM
jayeshv updated the diff for D8511: Change targettype in directory entry; add directory entry filter by name.

rebase

Wed, Sep 21, 3:05 PM
jayeshv updated the diff for D8511: Change targettype in directory entry; add directory entry filter by name.

review comments

Wed, Sep 21, 2:37 PM
jayeshv closed D8513: use 'directorySwhid' instead of 'swhid' while querying a DirectoryEntry.
Wed, Sep 21, 2:28 PM
jayeshv committed rDGQLad9602edd77c: use 'directorySwhid' instead of 'swhid' while querying a DirectoryEntry (authored by jayeshv).
use 'directorySwhid' instead of 'swhid' while querying a DirectoryEntry
Wed, Sep 21, 2:28 PM
jayeshv added inline comments to D8515: Add filters for latest visit status.
Wed, Sep 21, 2:27 PM
jayeshv updated the diff for D8515: Add filters for latest visit status.

address review comment

Wed, Sep 21, 2:24 PM
jayeshv requested review of D8515: Add filters for latest visit status.
Wed, Sep 21, 12:09 PM
jayeshv added a comment to D8511: Change targettype in directory entry; add directory entry filter by name.

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

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

Wed, Sep 21, 11:09 AM
jayeshv updated the summary of D8511: Change targettype in directory entry; add directory entry filter by name.
Wed, Sep 21, 9:53 AM
jayeshv updated the diff for D8511: Change targettype in directory entry; add directory entry filter by name.

typo

Wed, Sep 21, 9:52 AM
jayeshv updated the diff for D8511: Change targettype in directory entry; add directory entry filter by name.

add extra comment; update commit mesasge

Wed, Sep 21, 9:51 AM
jayeshv retitled D8513: use 'directorySwhid' instead of 'swhid' while querying a DirectoryEntry from rename argument 'swhid' to 'directorySwhid' for a directory entry object. to use 'directorySwhid' instead of 'swhid' while querying a DirectoryEntry.
Wed, Sep 21, 9:46 AM
jayeshv updated the diff for D8513: use 'directorySwhid' instead of 'swhid' while querying a DirectoryEntry.

change commit message

Wed, Sep 21, 9:45 AM

Tue, Sep 20

jayeshv requested review of D8513: use 'directorySwhid' instead of 'swhid' while querying a DirectoryEntry.
Tue, Sep 20, 6:47 PM
jayeshv updated the diff for D8511: Change targettype in directory entry; add directory entry filter by name.

revert generic filters

Tue, Sep 20, 4:40 PM
jayeshv updated the diff for D8511: Change targettype in directory entry; add directory entry filter by name.

add generic filter for connections

Tue, Sep 20, 4:06 PM
jayeshv requested review of D8511: Change targettype in directory entry; add directory entry filter by name.
Tue, Sep 20, 3:10 PM
jayeshv created P1455 get content query.
Tue, Sep 20, 10:04 AM

Wed, Sep 14

jayeshv closed D8473: Add missing parent object types to resolvers.
Wed, Sep 14, 3:48 PM
jayeshv committed rDGQLebc6e4453490: Add missing parent object types to resolvers (authored by jayeshv).
Add missing parent object types to resolvers
Wed, Sep 14, 3:48 PM
jayeshv requested review of D8473: Add missing parent object types to resolvers.
Wed, Sep 14, 12:19 PM

Tue, Sep 13

jayeshv committed rDGQLccf8b293afd2: Remove a duplicate instruction (authored by jayeshv).
Remove a duplicate instruction
Tue, Sep 13, 6:05 PM
jayeshv closed D8462: Add type hints to resolver modules.
Tue, Sep 13, 4:21 PM
jayeshv committed rDGQLf5ef0a1f8b4c: Add type hints to resolver modules (authored by jayeshv).
Add type hints to resolver modules
Tue, Sep 13, 4:21 PM
jayeshv updated the diff for D8462: Add type hints to resolver modules.

rebase

Tue, Sep 13, 4:17 PM
jayeshv added a comment to D8462: Add type hints to resolver modules.

Looks good to me, just one missing return type to add.

Reading you code more deeply, I think it would be better to get rid of the resolvers_factory module
as it adds a not really needed indirection and makes code hard to follow.

Returning the proper node object directly in the *_resolver functions seems simpler to me and
you can set a local mapping based on object type in them when returning an Union.

Tue, Sep 13, 4:13 PM
jayeshv updated the diff for D8462: Add type hints to resolver modules.

add missing return; from review comments

Tue, Sep 13, 4:07 PM
jayeshv closed D8465: Move uvicorn and starlette to the main requirement file.
Tue, Sep 13, 3:18 PM
jayeshv committed rDGQLf2f222267ff4: Move uvicorn and starlette to the main requirement file (authored by jayeshv).
Move uvicorn and starlette to the main requirement file
Tue, Sep 13, 3:18 PM
jayeshv requested review of D8462: Add type hints to resolver modules.
Tue, Sep 13, 2:35 PM
jayeshv added a revision to T4261: Swh-graphql Code cleanup: D8462: Add type hints to resolver modules.
Tue, Sep 13, 2:32 PM · GraphQL API
jayeshv triaged T4532: GraphQL: staging - Deploy version v0.0.4 as Normal priority.
Tue, Sep 13, 11:33 AM · System administration, GraphQL API
jayeshv closed D8457: Add type hints to archive and search modules.
Tue, Sep 13, 11:20 AM
jayeshv committed rDGQL7e4eacb71f64: Add type hints to archive and search modules (authored by jayeshv).
Add type hints to archive and search modules
Tue, Sep 13, 11:20 AM
jayeshv requested review of D8457: Add type hints to archive and search modules.
Tue, Sep 13, 11:12 AM
jayeshv added a revision to T4261: Swh-graphql Code cleanup: D8457: Add type hints to archive and search modules.
Tue, Sep 13, 11:09 AM · GraphQL API

Mon, Sep 12

jayeshv updated the task description for T4518: GraphQL: Handle missing reference object errors.
Mon, Sep 12, 4:49 PM · GraphQL API
jayeshv updated the task description for T4434: GraphQL: Setup a better query explorer.
Mon, Sep 12, 4:47 PM · GraphQL API
jayeshv lowered the priority of T4295: Support existing REST APIs in GraphQL from Normal to Low.
Mon, Sep 12, 4:44 PM · GraphQL API
jayeshv lowered the priority of T4293: Add generic filters and sort order on GraphQL connections from Normal to Low.
Mon, Sep 12, 4:44 PM · GraphQL API
jayeshv renamed T4299: GraphQL: Static query validation and max cost limiting from GraphQL: Basic query validation and rate limiting to GraphQL: Static query validation and max cost limiting.
Mon, Sep 12, 4:28 PM · GraphQL API
jayeshv added inline comments to D8425: [WIP]: Return more information with revision and release dates.
Mon, Sep 12, 2:43 PM
jayeshv closed D8426: Return none for missing snapshot in a visit status.
Mon, Sep 12, 2:37 PM
jayeshv closed T4514: graphql: object_id must be <class 'bytes'> (got None that is a <class 'NoneType'>) as Resolved by committing rDGQL2c3b16f7952b: Return none for missing snapshot in a visit status.
Mon, Sep 12, 2:37 PM · GraphQL API
jayeshv committed rDGQL2c3b16f7952b: Return none for missing snapshot in a visit status (authored by jayeshv).
Return none for missing snapshot in a visit status
Mon, Sep 12, 2:37 PM
jayeshv updated the diff for D8426: Return none for missing snapshot in a visit status.

improve tests from review comment

Mon, Sep 12, 2:28 PM
jayeshv added inline comments to D8425: [WIP]: Return more information with revision and release dates.
Mon, Sep 12, 2:23 PM
jayeshv added inline comments to D8425: [WIP]: Return more information with revision and release dates.
Mon, Sep 12, 12:28 PM
jayeshv updated the diff for D8425: [WIP]: Return more information with revision and release dates.

use a timestamp instead of seconds and microseconds

Mon, Sep 12, 12:26 PM
jayeshv retitled D8425: [WIP]: Return more information with revision and release dates from Return more information with revision and release dates to [WIP]: Return more information with revision and release dates.
Mon, Sep 12, 11:56 AM
jayeshv updated the diff for D8425: [WIP]: Return more information with revision and release dates.

Add types and support for None date

Mon, Sep 12, 11:53 AM
jayeshv added inline comments to D8426: Return none for missing snapshot in a visit status.
Mon, Sep 12, 10:53 AM
jayeshv updated the diff for D8426: Return none for missing snapshot in a visit status.

removed a unit test

Mon, Sep 12, 10:50 AM

Fri, Sep 9

jayeshv retitled D8426: Return none for missing snapshot in a visit status from Return sensible errors for missing snapshots in visit status to Return none for missing snapshot in a visit status.
Fri, Sep 9, 5:34 PM
jayeshv updated the diff for D8426: Return none for missing snapshot in a visit status.

commit message update

Fri, Sep 9, 5:33 PM
jayeshv updated the diff for D8426: Return none for missing snapshot in a visit status.

return None for snapshot instead of an Object

Fri, Sep 9, 5:18 PM
jayeshv added a comment to D8426: Return none for missing snapshot in a visit status.

Here is a way to do it:

diff --git a/swh/graphql/resolvers/resolvers.py b/swh/graphql/resolvers/resolvers.py
index 60992bb..1b0e93f 100644
--- a/swh/graphql/resolvers/resolvers.py
+++ b/swh/graphql/resolvers/resolvers.py
@@ -93,6 +93,8 @@ def snapshot_resolver(
 def visit_snapshot_resolver(
     obj, info: GraphQLResolveInfo, **kw
 ) -> rs.snapshot.VisitSnapshotNode:
+    if obj.snapshotSWHID is None:
+        return None
     resolver = get_node_resolver("visit-snapshot")
     return resolver(obj, info, **kw)
 
diff --git a/swh/graphql/tests/functional/test_visit_status.py b/swh/graphql/tests/functional/test_visit_status.py
index a250a9a..d83861a 100644
--- a/swh/graphql/tests/functional/test_visit_status.py
+++ b/swh/graphql/tests/functional/test_visit_status.py
@@ -28,7 +28,7 @@ def test_get_visit_status(client):
         visit.origin,
         visit.visit,
     )
-    data, _ = get_query_response(client, query_str)
+    data, err = get_query_response(client, query_str)
     result_status = get_visit_status()[3]
     assert data["visit"]["status"]["nodes"][0] == {
         "date": result_status.date.isoformat(),
@@ -36,6 +36,7 @@ def test_get_visit_status(client):
         "status": result_status.status,
         "type": result_status.type,
     }
+    assert err is None
 
 
 def test_get_visit_missing_snapshot(client):
@@ -67,7 +68,4 @@ def test_get_visit_missing_snapshot(client):
         "status": result_status.status,
         "type": result_status.type,
     }
-    assert len(err) == 1
-    assert (
-        "Cannot return null for non-nullable field Snapshot.swhid" in err[0]["message"]
-    )
+    assert err is None
Fri, Sep 9, 5:04 PM
jayeshv updated the diff for D8426: Return none for missing snapshot in a visit status.

update error to raise

Fri, Sep 9, 3:52 PM
jayeshv added a comment to D8426: Return none for missing snapshot in a visit status.

Sure, but in that example, the error is because for some reason, they could not get the value of name for an object that exists. The error in the example even reflects this, as it mentions "character with ID 1002".

In our case, that object does not exist at all.

Fri, Sep 9, 3:25 PM
jayeshv added a comment to D8426: Return none for missing snapshot in a visit status.

The section you linked mentions null values in Non-Null fields, but not of null values for objects with a Non-Null field.

Are you sure this is disallowed by the standard, not "only" a bug in the GraphQL library we use?

Fri, Sep 9, 3:04 PM
jayeshv added a comment to D8426: Return none for missing snapshot in a visit status.

The section you linked mentions null values in Non-Null fields, but not of null values for objects with a Non-Null field.

Are you sure this is disallowed by the standard, not "only" a bug in the GraphQL library we use?

Fri, Sep 9, 2:51 PM
jayeshv added a comment to D8426: Return none for missing snapshot in a visit status.

The issue is snapshot has non nullable fields (like SWHID) and GraphQL standard will not allow a
null for an object with non nullable field without an error.

I can't find what part of the standard disallows it, nor any mention one way or another outside the standard. Any pointer?

Fri, Sep 9, 2:34 PM
jayeshv added a comment to D8426: Return none for missing snapshot in a visit status.
> I do not get the issue here, using the following changes on top of your diff, it works as expected:
>

It will work as expected in the data part. But there will be an error associated, that we think is unnecessary.
With your changes, it will be a "NoneType' object has no attribute 'object_id" instead of an ObjectError

Could you write a snippet query to reproduce the error ? I still do not get what's wrong here.

Fri, Sep 9, 2:12 PM
jayeshv added a comment to D8426: Return none for missing snapshot in a visit status.
> I do not get the issue here, using the following changes on top of your diff, it works as expected:
>

It will work as expected in the data part. But there will be an error associated, that we think is unnecessary.
With your changes, it will be a "NoneType' object has no attribute 'object_id" instead of an ObjectError

Fri, Sep 9, 2:07 PM
jayeshv triaged T4518: GraphQL: Handle missing reference object errors as Low priority.
Fri, Sep 9, 12:41 PM · GraphQL API
jayeshv added a comment to D8426: Return none for missing snapshot in a visit status.

Why have two different types, instead of making the snapshot field nullable, like this?

type VisitStatus {
  status: String!
  date: DateTime!
  type: String
  snapshot: Snapshot
}
Fri, Sep 9, 12:35 PM
jayeshv added a comment to D8426: Return none for missing snapshot in a visit status.

@vlorentz The right fix is to add a new visit status type to the schema like below.

Fri, Sep 9, 12:16 PM
jayeshv closed T4508: GraphQL: Add a directoryEntry entry point, a subtask of T4083: New public API (GraphQL + thin layer), as Resolved.
Fri, Sep 9, 12:00 PM · GraphQL API, Roadmap 2022
jayeshv closed T4508: GraphQL: Add a directoryEntry entry point as Resolved.
Fri, Sep 9, 12:00 PM · GraphQL API
jayeshv closed D8435: Add revision object as a possible directory entry target.
Fri, Sep 9, 11:59 AM
jayeshv committed rDGQL6e5d34e94de1: Add revision object as a possible directory entry target (authored by jayeshv).
Add revision object as a possible directory entry target
Fri, Sep 9, 11:59 AM
jayeshv requested review of D8435: Add revision object as a possible directory entry target.
Fri, Sep 9, 11:57 AM
jayeshv closed D8396: Add a directoryEntry entrypoint.
Fri, Sep 9, 11:47 AM
jayeshv committed rDGQL1af442fd62fd: Add a directoryEntry entrypoint (authored by jayeshv).
Add a directoryEntry entrypoint
Fri, Sep 9, 11:47 AM
jayeshv added inline comments to D8396: Add a directoryEntry entrypoint.
Fri, Sep 9, 11:46 AM
jayeshv updated the diff for D8396: Add a directoryEntry entrypoint.

remove ununsed test

Fri, Sep 9, 11:21 AM
jayeshv added inline comments to D8396: Add a directoryEntry entrypoint.
Fri, Sep 9, 11:16 AM