Page MenuHomeSoftware Heritage
Differential D8310

Add revision log field in the revision type
ClosedPublic
Actions

Authored by jayeshv on Aug 24 2022, 5:27 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T4366: GraphQL: add revision log to the revision type
Commits
rDGQLa31419d74c45: Add revision log field in the revision type
Summary

Storage is returning a list of dicts for the revision log
Add a from_dict converter to create the model objects from these dicts
This is not efficient and adds an extra iteration. Has to be
fixed in the storage in the future.

Related to T4366

Diff Detail

Repository
rDGQL GraphQL API
Branch
revision-log
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 31043
Build 48566: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 48565: arc lint + arc unit

Event Timeline

jayeshv created this revision.Aug 24 2022, 5:27 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptAug 24 2022, 5:27 PM
swh-public-ci added a comment.Aug 24 2022, 5:29 PM

Build is green

Patch application report for D8310 (id=29993)

Rebasing onto 6fb1913ff9...

Current branch diff-target is up to date.
Changes applied before test
commit e21947ad862f0a75edbff1c87607d85e01a61fe9
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Aug 24 17:04:56 2022 +0200

    Add revision log field in the revision type
    
    Storage is returning a list of dicts for the revision log
    Add a from_dict converter to create the model objects from these dicts
    This is not efficient and adds an extra iteration. Has to be
    fixed in the storage in the future.
    
    Related to T4366

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

Harbormaster completed remote builds in B31043: Diff 29993.Aug 24 2022, 5:29 PM
jayeshv requested review of this revision.Aug 24 2022, 5:29 PM
jayeshv updated this revision to Diff 29994.Aug 24 2022, 5:30 PM

Remove unnecessary comment

swh-public-ci added a comment.Aug 24 2022, 5:32 PM

Build is green

Patch application report for D8310 (id=29994)

Rebasing onto 6fb1913ff9...

Current branch diff-target is up to date.
Changes applied before test
commit 12003a050365d3570b4e3d8ba9b0bf34810b0a1d
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Aug 24 17:04:56 2022 +0200

    Add revision log field in the revision type
    
    Storage is returning a list of dicts for the revision log
    Add a from_dict converter to create the model objects from these dicts
    This is not efficient and adds an extra iteration. Has to be
    fixed in the storage in the future.
    
    Related to T4366

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

Harbormaster completed remote builds in B31044: Diff 29994.Aug 24 2022, 5:32 PM
jayeshv retitled this revision from Add revision log field in the revision type to [WIP]Add revision log field in the revision type.Aug 25 2022, 11:45 AM
jayeshv edited the summary of this revision. (Show Details)
vlorentz retitled this revision from [WIP]Add revision log field in the revision type to [WIP] Add revision log field in the revision type.Aug 25 2022, 11:54 AM
jayeshv updated this revision to Diff 30030.Aug 25 2022, 3:16 PM

functional tests

swh-public-ci added a comment.Aug 25 2022, 3:18 PM

Build is green

Patch application report for D8310 (id=30030)

Rebasing onto 40c0debad7...

Current branch diff-target is up to date.
Changes applied before test
commit e3fa1bbb5cdbb468a56b11ca408b0d4b78f95a90
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Aug 24 17:04:56 2022 +0200

    Add revision log field in the revision type
    
    Storage is returning a list of dicts for the revision log.
    Add a from_dict converter to create the model objects from these dicts
    This is not efficient and adds an extra iteration. Has to be
    fixed in the storage in the future.
    
    Related to T4366

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

Harbormaster completed remote builds in B31080: Diff 30030.Aug 25 2022, 3:18 PM
jayeshv retitled this revision from [WIP] Add revision log field in the revision type to Add revision log field in the revision type.Aug 25 2022, 3:18 PM
jayeshv edited the summary of this revision. (Show Details)
ardumont accepted this revision.Aug 26 2022, 2:23 PM
ardumont added a subscriber: ardumont.

lgtm

swh/graphql/resolvers/revision.py
105
swh/graphql/tests/data.py
87 ↗(On Diff #30030)

is it necessary to hard-code that id here?
If the revision object does not declare it, it will be recomputed once it's accessed, it might be better that way (same goes for other dag model object).

This revision is now accepted and ready to land.Aug 26 2022, 2:23 PM
jayeshv added inline comments.Aug 26 2022, 2:30 PM
swh/graphql/tests/data.py
87 ↗(On Diff #30030)

If I let that compute the SWHID, how do I use the same object while testing?
This revision is used in both "test_get_revision_log" and "test_get_revision_parents" functions below.

ardumont added inline comments.Aug 26 2022, 2:33 PM
swh/graphql/tests/data.py
87 ↗(On Diff #30030)

You can have the objects be declared at the module level and then refer to them both in function (and/or fixture) and in test.

swh/graphql/tests/functional/test_revision.py
133 ↗(On Diff #30030)

i guess? (it's probably missing the hash to hex yadi yada though)

jayeshv updated this revision to Diff 30057.Aug 26 2022, 2:34 PM

fix typo

swh-public-ci added a comment.Aug 26 2022, 2:36 PM

Build is green

Patch application report for D8310 (id=30057)

Rebasing onto 45582e79ee...

Current branch diff-target is up to date.
Changes applied before test
commit a31419d74c4562f3f9d4dad02a7b8ad1590aa97b
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Aug 24 17:04:56 2022 +0200

    Add revision log field in the revision type
    
    Storage is returning a list of dicts for the revision log.
    Add a from_dict converter to create the model objects from these dicts
    This is not efficient and adds an extra iteration. Has to be
    fixed in the storage in the future.
    
    Related to T4366

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

Harbormaster completed remote builds in B31104: Diff 30057.Aug 26 2022, 2:36 PM
Closed by commit rDGQLa31419d74c45: Add revision log field in the revision type (authored by jayeshv). · Explain WhyAug 26 2022, 2:39 PM
This revision was automatically updated to reflect the committed changes.
jayeshv added a commit: rDGQLa31419d74c45: Add revision log field in the revision type.
anlambert added a subscriber: anlambert.Aug 26 2022, 2:39 PM
anlambert added inline comments.
swh/graphql/tests/data.py
87 ↗(On Diff #30030)

I agree with ardumont, hard-coding SWHIDs could be avoided here.

You should wrap that revision in a fixture (without declaring the id as it is automatically computed when accessing the field)
and pass it as parameter of the tests that need it.

jayeshv added inline comments.Aug 26 2022, 2:42 PM
swh/graphql/tests/data.py
87 ↗(On Diff #30030)

Got it, Thanks. I will make another diff for those change. This I will land

87 ↗(On Diff #30030)

Thanks, I will do that

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
graphql/
resolvers/
8 lines98%
Loading...
6 lines85%
Loading...

Diff 29993

View Options

swh/graphql/resolvers/resolvers.py

Loading...
View Options

swh/graphql/resolvers/revision.py

Loading...
About · Developer information · Legal