Page MenuHomeSoftware Heritage
Differential D7653

Add missing `content_git_object`
ClosedPublic
Actions

Authored by Ericson2314 on Apr 26 2022, 7:42 AM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDMOD08c69e63794a: Add missing `content_git_object`
Required Signatures
L3 Software Heritage Contributor License Agreement, version 1.0
Summary

Add missing content_git_object

This would be useful for the IPFS bridge, and seems good to complete the
API in any sense.

Diff Detail

Repository
rDMOD Data model
Branch
arcpatch-D7653 (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 28748
Build 44927: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 44926: arc lint + arc unit

Event Timeline

Ericson2314 created this revision.Apr 26 2022, 7:42 AM
Herald added a required legal document: L3 Software Heritage Contributor License Agreement, version 1.0. · View Herald TranscriptApr 26 2022, 7:42 AM
swh-public-ci added a comment.Apr 26 2022, 7:43 AM

Build has FAILED

Patch application report for D7653 (id=27689)

Rebasing onto f82a2179dc...

Current branch diff-target is up to date.
Changes applied before test
commit 3bc6f9f4c1303e7e12d45e8fb5c95054d541e0fc
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: plt-amy, zack, anlambert, vlorentz

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/472/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/472/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 7:43 AM
Harbormaster failed remote builds in B28746: Diff 27689!
Ericson2314 updated this revision to Diff 27690.Apr 26 2022, 7:47 AM

Check data is non-None

swh-public-ci added a comment.Apr 26 2022, 7:48 AM

Build has FAILED

Patch application report for D7653 (id=27690)

Rebasing onto f82a2179dc...

Current branch diff-target is up to date.
Changes applied before test
commit fd7c5e10e5587b304cec2f9f2a19bfd55b7d0894
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/473/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/473/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 7:48 AM
Harbormaster failed remote builds in B28747: Diff 27690!
Ericson2314 updated this revision to Diff 27691.Apr 26 2022, 7:50 AM

self -> content, model.MissingData

swh-public-ci added a comment.Apr 26 2022, 7:50 AM

Build has FAILED

Patch application report for D7653 (id=27691)

Rebasing onto f82a2179dc...

Current branch diff-target is up to date.
Changes applied before test
commit efd5a9e521bfc267b5af4a93e57b9337bd5cfb50
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/474/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/474/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 7:50 AM
Harbormaster failed remote builds in B28748: Diff 27691!
Ericson2314 updated this revision to Diff 27692.Apr 26 2022, 7:51 AM

model.MissingData

swh-public-ci added a comment.Apr 26 2022, 7:53 AM

Build has FAILED

Patch application report for D7653 (id=27692)

Rebasing onto f82a2179dc...

Current branch diff-target is up to date.
Changes applied before test
commit c1a50cc7bcdccfb3dca6d497b996c66e67331e1b
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/475/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/475/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 7:53 AM
Harbormaster failed remote builds in B28749: Diff 27692!
Ericson2314 mentioned this in D7192: Route for fetching Git-encoded objects.Apr 26 2022, 7:56 AM
vlorentz added a comment.Apr 26 2022, 11:50 AM

HashableObject is for objects with an id, which is their only hash. This makes sense for most objects, because they only have one hash algorithm we care about
For Content, we do care about the other hash, and sha1_git is not more important than the others.

You should skip testing content.test(), and only test content_git_object directly

Ericson2314 updated this revision to Diff 27798.Apr 26 2022, 8:35 PM

Test content_git_object directly

swh-public-ci added a comment.Apr 26 2022, 8:36 PM

Build has FAILED

Patch application report for D7653 (id=27798)

Rebasing onto 2c0701cb77...

First, rewinding head to replay your work on top of it...
Applying: Add missing `content_git_object`
Changes applied before test
commit 277764247ea3e1f3af3be4be8d9f999dcf8b615b
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/477/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/477/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 8:36 PM
Harbormaster failed remote builds in B28858: Diff 27798!
Ericson2314 updated this revision to Diff 27799.Apr 26 2022, 8:37 PM

Fix silly typo!

swh-public-ci added a comment.Apr 26 2022, 8:37 PM

Build has FAILED

Patch application report for D7653 (id=27799)

Rebasing onto 2c0701cb77...

First, rewinding head to replay your work on top of it...
Applying: Add missing `content_git_object`
Changes applied before test
commit b039e2d750b724368aa812b319755ff2344f6d7e
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    On the other hand, I don't think this test is correct at all. `Content`
    doesn't derive from `HashableObject` like the others. I suspect that is
    why `content_git_object` didn't already exist.
    
    I get the sense there are number of historical subtleties here I am
    probably stomping over in ignorance :).
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/478/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/478/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 8:37 PM
Harbormaster failed remote builds in B28859: Diff 27799!
Ericson2314 updated this revision to Diff 27800.Apr 26 2022, 8:40 PM

Fix another silly typo

swh-public-ci added a comment.Apr 26 2022, 8:42 PM

Build has FAILED

Patch application report for D7653 (id=27800)

Rebasing onto 2c0701cb77...

Current branch diff-target is up to date.
Changes applied before test
commit 57c66993b6e7587b9f92b7007aab44c6719cd9b2
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/479/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/479/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 26 2022, 8:42 PM
Harbormaster failed remote builds in B28860: Diff 27800!
Ericson2314 updated this revision to Diff 27801.Apr 26 2022, 8:50 PM

Need to hash raw object to compare!

swh-public-ci added a comment.Apr 26 2022, 8:52 PM

Build is green

Patch application report for D7653 (id=27801)

Rebasing onto 2c0701cb77...

Current branch diff-target is up to date.
Changes applied before test
commit 524f2324690a9459d842bcffa3991c52572cd1d0
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    Summary:
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.
    
    Reviewers: #reviewers
    
    Subscribers: vlorentz, anlambert, zack, plt-amy
    
    Differential Revision: https://forge.softwareheritage.org/D7653

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

Harbormaster completed remote builds in B28861: Diff 27801.Apr 26 2022, 8:52 PM
Ericson2314 requested review of this revision.Apr 26 2022, 8:52 PM
Ericson2314 edited the summary of this revision. (Show Details)Apr 26 2022, 8:53 PM

Thanks @vlorentz for all your advice and help. I think this is good now!

vlorentz added a comment.Apr 27 2022, 5:44 PM

Thanks.

One last thing for this diff: could you clean up the commit message? It should be this:

Add missing `content_git_object`

This would be useful for the IPFS bridge, and seems good to complete the
API in any sense.
Ericson2314 edited the summary of this revision. (Show Details)Apr 27 2022, 5:51 PM
Ericson2314 added a comment.Apr 27 2022, 5:54 PM

@vlorentz do you mean repeat the title in the summary field?

(I suppose https://docs.softwareheritage.org/devel/contributing/phabricator.html#starting-a-new-feature-and-submit-it-for-review did say that but I was bit confused as it seemed like the title might appear twice in the final commit.)

vlorentz added a comment.EditedApr 27 2022, 6:11 PM

No, remove the fields. ie. replace all of this:

Add missing `content_git_object`

Summary:
This would be useful for the IPFS bridge, and seems good to complete the
API in any sense.

Reviewers: #reviewers

Subscribers: vlorentz, anlambert, zack, plt-amy

Differential Revision: https://forge.softwareheritage.org/D7653

with just this:

Add missing `content_git_object`

This would be useful for the IPFS bridge, and seems good to complete the
API in any sense.

Summary/Test Plan/Reviewers/Subscribers/... is only the diff's message, not the commit's

Ericson2314 updated this revision to Diff 27854.Apr 27 2022, 6:17 PM

Use git-style not phab-style commit message

swh-public-ci added a comment.Apr 27 2022, 6:20 PM

Build is green

Patch application report for D7653 (id=27854)

Rebasing onto 2c0701cb77...

Current branch diff-target is up to date.
Changes applied before test
commit 08c69e63794a6d7a839f7201ee957c223f65f898
Author: John Ericson <John.Ericson@Obsidian.Systems>
Date:   Tue Apr 26 01:33:07 2022 -0400

    Add missing `content_git_object`
    
    This would be useful for the IPFS bridge, and seems good to complete the
    API in any sense.

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

Harbormaster completed remote builds in B28915: Diff 27854.Apr 27 2022, 6:20 PM
vlorentz accepted this revision.Apr 27 2022, 6:23 PM

Thanks! Now you can push to master

This revision is now accepted and ready to land.Apr 27 2022, 6:23 PM
Closed by commit rDMOD08c69e63794a: Add missing `content_git_object` (authored by Ericson2314). · Explain WhyApr 27 2022, 6:28 PM
This revision was automatically updated to reflect the committed changes.
Ericson2314 added a commit: rDMOD08c69e63794a: Add missing `content_git_object`.

Revision Contents
Changeset List

PathSize
swh/
model/
13 lines
tests/
14 lines

Diff 27691

View Options

swh/model/git_objects.py

Loading...
View Options

swh/model/tests/test_model.py

Loading...
About · Developer information · Legal