Page MenuHomeSoftware Heritage
Differential D8425

Return more information with revision and release dates
ClosedPublic
Actions

Authored by jayeshv on Sep 8 2022, 3:40 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T4365: GraphQL: Use the right date time serialization
Commits
rDGQL5446d4f87f85: Return more information with revision and release dates
Summary

Return more information with revision and release dates

Revision and release dates could have different values for offset.
Add a new schema type to handle these two dates and return available
offset as a binary string.

Related to T4365

Diff Detail

Repository
rDGQL GraphQL API
Branch
timezone
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 31398
Build 49118: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 49117: arc lint + arc unit

Event Timeline

jayeshv created this revision.Sep 8 2022, 3:40 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptSep 8 2022, 3:40 PM
jayeshv updated this revision to Diff 30377.Sep 8 2022, 3:41 PM

remove unused doc

swh-public-ci added a comment.Sep 8 2022, 3:42 PM

Build is green

Patch application report for D8425 (id=30376)

Rebasing onto 467e0b7e28...

Current branch diff-target is up to date.
Changes applied before test
commit 124ea6de94268cbf71354b6e89c82f715d7dde3c
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Not all revision and release dates are convertable to datetime.
    Add a new schema type to handle these two dates and return all the
    availalble data.
    
    Related to T4365

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

Harbormaster completed remote builds in B31397: Diff 30376.Sep 8 2022, 3:42 PM
swh-public-ci added a comment.Sep 8 2022, 3:43 PM

Build is green

Patch application report for D8425 (id=30377)

Rebasing onto 467e0b7e28...

Current branch diff-target is up to date.
Changes applied before test
commit 79660a4c7058d7471d8ff90ad21cb4a166fa18c7
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Not all revision and release dates are convertable to datetime.
    Add a new schema type to handle these two dates and return all the
    availalble data.
    
    Related to T4365

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

Harbormaster completed remote builds in B31398: Diff 30377.Sep 8 2022, 3:43 PM
jayeshv requested review of this revision.Sep 8 2022, 3:43 PM
vlorentz added a subscriber: vlorentz.Sep 12 2022, 10:39 AM
vlorentz added inline comments.
swh/graphql/resolvers/scalars.py
32

use isinstance

38

What other types can value be?

swh/graphql/schema/schema.graphql
526

the doc should explain why there are both an ISO8601 date and timestamp + offset.

528

why not just Date?

jayeshv updated this revision to Diff 30439.Sep 12 2022, 11:53 AM

Add types and support for None date

swh-public-ci added a comment.Sep 12 2022, 11:56 AM

Build is green

Patch application report for D8425 (id=30439)

Rebasing onto 6e5d34e94d...

First, rewinding head to replay your work on top of it...
Applying: Return more information with revision and release dates
Changes applied before test
commit 09665f45133082d4a0447022bd1b0f921f180e50
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Not all revision and release dates are convertable to datetime.
    Add a new schema type to handle these two dates and return all the
    availalble data.
    
    Related to T4365

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

Harbormaster completed remote builds in B31455: Diff 30439.Sep 12 2022, 11:56 AM
jayeshv retitled this revision from Return more information with revision and release dates to [WIP]: Return more information with revision and release dates.Sep 12 2022, 11:56 AM
jayeshv updated this revision to Diff 30441.Sep 12 2022, 12:26 PM
jayeshv marked 2 inline comments as done.

use a timestamp instead of seconds and microseconds

jayeshv marked an inline comment as done.Sep 12 2022, 12:28 PM
jayeshv added inline comments.
swh/graphql/resolvers/resolvers.py
314

@vlorentz changed this to use a timestamp instead of seconds and microseconds. What do you think?

swh/graphql/resolvers/scalars.py
38

Can be an instance of TimestampWithTimezone or a python datetime. Any other type will return a None.
I don't really know the input (value) can be None, but that is anyway supported.

swh-public-ci added a comment.Sep 12 2022, 12:29 PM

Build is green

Patch application report for D8425 (id=30441)

Rebasing onto 6e5d34e94d...

First, rewinding head to replay your work on top of it...
Applying: Return more information with revision and release dates
Changes applied before test
commit 41d660696bc4268a1ba8f934ea51bec06967cbbb
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Not all revision and release dates are convertable to datetime.
    Add a new schema type to handle these two dates and return all the
    availalble data.
    
    Related to T4365

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

Harbormaster completed remote builds in B31457: Diff 30441.Sep 12 2022, 12:30 PM
vlorentz added inline comments.Sep 12 2022, 1:33 PM
swh/graphql/resolvers/resolvers.py
314

Using floats loses precision for very high or low timestamps; but I guess that's fine

swh/graphql/resolvers/scalars.py
38

why not two different functions, one for each type?

jayeshv added inline comments.Sep 12 2022, 2:23 PM
swh/graphql/resolvers/scalars.py
38

This same function is used to serialize datetimes from both user generated (date field in the Date object) and db dates (date in revision object).
adding two different scalars for both types is perfectly fine, but wouldn't that make a client a bit more complex? (I was trying to keep the number of scalars to a minimum)

jayeshv added inline comments.Sep 12 2022, 2:43 PM
swh/graphql/resolvers/scalars.py
38

Fix the typo

This same function is used to serialize datetimes from both user generated (date field in the Date object) and db dates (date in the visit status object).
adding two different scalars for both types is perfectly fine, but wouldn't that make a client a bit more complex? (I was trying to keep the number of scalars to a minimum)

jayeshv updated this revision to Diff 31584.Oct 25 2022, 9:26 AM

test fixes

swh-public-ci added a comment.Oct 25 2022, 9:29 AM

Build is green

Patch application report for D8425 (id=31584)

Rebasing onto 7522a848fa...

Current branch diff-target is up to date.
Changes applied before test
commit 85c9bf459f242c103e46946cb043d66d0648b59f
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Not all revision and release dates are convertable to datetime.
    Add a new schema type to handle these two dates and return all the
    availalble data.
    
    Related to T4365

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

Harbormaster completed remote builds in B32538: Diff 31584.Oct 25 2022, 9:29 AM
jayeshv updated this revision to Diff 32118.Dec 2 2022, 3:39 PM

bug fix

swh-public-ci added a comment.Dec 2 2022, 3:44 PM

Build is green

Patch application report for D8425 (id=32118)

Rebasing onto c28b65feaf...

Current branch diff-target is up to date.
Changes applied before test
commit 5e8890d818d11314c674bb742898905ac5bdce92
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Not all revision and release dates are convertable to datetime.
    Add a new schema type to handle these two dates and return all the
    availalble data.
    
    Related to T4365

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

Harbormaster completed remote builds in B33071: Diff 32118.Dec 2 2022, 3:44 PM
jayeshv retitled this revision from [WIP]: Return more information with revision and release dates to Return more information with revision and release dates.Dec 12 2022, 3:32 PM
jayeshv updated this revision to Diff 32323.Dec 20 2022, 10:10 AM

rebase

jayeshv retitled this revision from Return more information with revision and release dates to [WIP] Return more information with revision and release dates.Dec 20 2022, 10:10 AM
swh-public-ci added a comment.Dec 20 2022, 10:12 AM

Build is green

Patch application report for D8425 (id=32323)

Rebasing onto c8bc6947fa...

First, rewinding head to replay your work on top of it...
Applying: Return more information with revision and release dates
Changes applied before test
commit d389e69c86db19bed90938384e2e638120fed3b9
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Not all revision and release dates are convertable to datetime.
    Add a new schema type to handle these two dates and return all the
    availalble data.
    
    Related to T4365

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

Harbormaster completed remote builds in B33314: Diff 32323.Dec 20 2022, 10:12 AM
jayeshv updated this revision to Diff 32332.Dec 20 2022, 4:00 PM

Change offset to a binary string

jayeshv edited the summary of this revision. (Show Details)Dec 20 2022, 4:00 PM
swh-public-ci added a comment.Dec 20 2022, 4:02 PM

Build is green

Patch application report for D8425 (id=32332)

Rebasing onto c8bc6947fa...

First, rewinding head to replay your work on top of it...
Applying: Return more information with revision and release dates
Changes applied before test
commit ec9873527f0afbb8e500a51173d2b2b847f536e5
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Revision and release dates could have different values for offset.
    Add a new schema type to handle these two dates and return available
    offset as a binary string.
    
    Related to T4365

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

Harbormaster completed remote builds in B33323: Diff 32332.Dec 20 2022, 4:02 PM
jayeshv retitled this revision from [WIP] Return more information with revision and release dates to Return more information with revision and release dates.Dec 20 2022, 4:03 PM
jayeshv updated this revision to Diff 32333.Dec 20 2022, 4:10 PM

Add missing test

swh-public-ci added a comment.Dec 20 2022, 4:12 PM

Build is green

Patch application report for D8425 (id=32333)

Rebasing onto c8bc6947fa...

First, rewinding head to replay your work on top of it...
Applying: Return more information with revision and release dates
Changes applied before test
commit 49ee60bf79d6973b61b76d07033f4e0911148930
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Revision and release dates could have different values for offset.
    Add a new schema type to handle these two dates and return available
    offset as a binary string.
    
    Related to T4365

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

Harbormaster completed remote builds in B33324: Diff 32333.Dec 20 2022, 4:12 PM
anlambert requested changes to this revision.Dec 21 2022, 11:04 AM
anlambert added a subscriber: anlambert.

LGTM but you also need to wrap the committer_date field from Revision model.

swh/graphql/schema/schema.graphql
582

I just noticed that you forgot to wrap the commiter_date field from the revision model.
You should add it as that info is important to have.

This revision now requires changes to proceed.Dec 21 2022, 11:04 AM
jayeshv updated this revision to Diff 32340.Dec 21 2022, 11:19 AM

Add committer_date im the revision object

jayeshv marked an inline comment as done.Dec 21 2022, 11:20 AM
jayeshv added inline comments.
swh/graphql/schema/schema.graphql
582

Thanks, added that.

swh-public-ci added a comment.Dec 21 2022, 11:21 AM

Build is green

Patch application report for D8425 (id=32340)

Rebasing onto c8bc6947fa...

First, rewinding head to replay your work on top of it...
Applying: Return more information with revision and release dates
Changes applied before test
commit 7995e5a4858a71a54a1ac853caabe3a357842215
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Revision and release dates could have different values for offset.
    Add a new schema type to handle these two dates and return available
    offset as a binary string.
    
    Related to T4365

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

Harbormaster completed remote builds in B33327: Diff 32340.Dec 21 2022, 11:21 AM
anlambert accepted this revision.Dec 21 2022, 12:03 PM
This revision is now accepted and ready to land.Dec 21 2022, 12:03 PM
jayeshv updated this revision to Diff 32341.Dec 21 2022, 12:15 PM
jayeshv marked an inline comment as done.

rebase

swh-public-ci added a comment.Dec 21 2022, 12:18 PM

Build is green

Patch application report for D8425 (id=32341)

Rebasing onto c8bc6947fa...

Current branch diff-target is up to date.
Changes applied before test
commit 5446d4f87f85fdb3b942e50e935dc2ef6c6e8d29
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Thu Sep 8 15:25:40 2022 +0200

    Return more information with revision and release dates
    
    Revision and release dates could have different values for offset.
    Add a new schema type to handle these two dates and return available
    offset as a binary string.
    
    Related to T4365

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

Harbormaster completed remote builds in B33328: Diff 32341.Dec 21 2022, 12:18 PM
Closed by commit rDGQL5446d4f87f85: Return more information with revision and release dates (authored by jayeshv). · Explain WhyDec 21 2022, 12:18 PM
This revision was automatically updated to reflect the committed changes.
jayeshv added a commit: rDGQL5446d4f87f85: Return more information with revision and release dates.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
graphql/
1 line100%
Loading...
resolvers/
22 lines100%
Loading...
11 lines100%
Loading...
schema/
33 lines--
tests/
functional/
8 lines100%
Loading...
15 lines100%
Loading...
unit/
resolvers/
35 lines100%
Loading...

Diff 30377

View Options

swh/graphql/app.py

Loading...
View Options

swh/graphql/resolvers/resolvers.py

Loading...
View Options

swh/graphql/resolvers/scalars.py

Loading...
View Options

swh/graphql/schema/schema.graphql

Loading...
View Options

swh/graphql/tests/functional/test_release_node.py

Loading...
View Options

swh/graphql/tests/functional/test_revision.py

Loading...
View Options

swh/graphql/tests/unit/resolvers/test_scalars.py

Loading...
About · Developer information · Legal