Page MenuHomeSoftware Heritage
Differential D7486

moderation: Fix deposit uri computation for 'code' deposits
ClosedPublic
Actions

Authored by ardumont on Apr 1 2022, 4:39 PM.

Details

Reviewers
moranegg
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T4123: Change uri in deposit admin view
Commits
rDWAPPS8ffd81760aa9: moderation: Fix deposit uri computation for 'code' deposits
Summary

Prior to this commit, this was done for all deposits but it's only a good fallback for
typed deposit 'code'.

Related to T4123

Test Plan

tox

Diff Detail

Repository
rDWAPPS Web applications
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 28071
Build 43962: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 43961: arc lint + arc unit

Event Timeline

ardumont created this revision.Apr 1 2022, 4:39 PM
ardumont updated this revision to Diff 27154.Apr 1 2022, 4:42 PM

Better implementation, it could be that raw_provenance is not provided so we need to
account for that nonetheless.

ardumont updated this revision to Diff 27155.Apr 1 2022, 4:48 PM

Explicit how the uri is computed

swh-public-ci added a comment.Apr 1 2022, 5:00 PM

Build is green

Patch application report for D7486 (id=27153)

Rebasing onto ad5add7d36...

Current branch diff-target is up to date.
Changes applied before test
commit 96772318471a05fe360ebfd2da4772ef8ad16a2d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Apr 1 16:37:42 2022 +0200

    moderation: Fix deposit uri computation for 'code' deposits
    
    Prior to this commit, this was done for all deposits but it's only a good fallback for
    typed deposit 'code'.
    
    Related to T4123

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

Harbormaster completed remote builds in B28069: Diff 27153.Apr 1 2022, 5:00 PM
swh-public-ci added a comment.Apr 1 2022, 5:04 PM

Build is green

Patch application report for D7486 (id=27154)

Rebasing onto ad5add7d36...

Current branch diff-target is up to date.
Changes applied before test
commit e05391bd248bf601c27822b1a7633579eb263c9d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Apr 1 16:37:42 2022 +0200

    moderation: Fix deposit uri computation for 'code' deposits
    
    Prior to this commit, this was done for all deposits but it's only a good fallback for
    typed deposit 'code'.
    
    Related to T4123

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

Harbormaster completed remote builds in B28070: Diff 27154.Apr 1 2022, 5:04 PM
swh-public-ci added a comment.Apr 1 2022, 5:07 PM

Build is green

Patch application report for D7486 (id=27155)

Rebasing onto ad5add7d36...

Current branch diff-target is up to date.
Changes applied before test
commit 8ffd81760aa971773b693a07934d717dfaf91b22
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Apr 1 16:37:42 2022 +0200

    moderation: Fix deposit uri computation for 'code' deposits
    
    Prior to this commit, this was done for all deposits but it's only a good fallback for
    typed deposit 'code'.
    
    Related to T4123

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

Harbormaster completed remote builds in B28071: Diff 27155.Apr 1 2022, 5:07 PM
ardumont requested review of this revision.Apr 1 2022, 5:08 PM
vlorentz accepted this revision.Apr 4 2022, 10:15 AM
This revision is now accepted and ready to land.Apr 4 2022, 10:15 AM
Closed by commit rDWAPPS8ffd81760aa9: moderation: Fix deposit uri computation for 'code' deposits (authored by ardumont). · Explain WhyApr 4 2022, 12:48 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDWAPPS8ffd81760aa9: moderation: Fix deposit uri computation for 'code' deposits.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
web/
admin/
23 lines25%
Loading...

Diff 27155

View Options

swh/web/admin/deposit.py

Show First 20 Lines • Show All 565 Lines • ▼ Show 20 Lines def test_content_add_collision(self):
cont1b = cont1.copy() cont1b = cont1.copy()
sha256_array = bytearray(cont1b['sha256']) sha256_array = bytearray(cont1b['sha256'])
sha256_array[0] += 1 sha256_array[0] += 1
cont1b['sha256'] = bytes(sha256_array) cont1b['sha256'] = bytes(sha256_array)
with self.assertRaises(HashCollision) as cm: with self.assertRaises(HashCollision) as cm:
self.storage.content_add([cont1, cont1b]) self.storage.content_add([cont1, cont1b])
self.assertEqual(cm.exception.args[0], 'sha1') self.assertIn(cm.exception.args[0], ['sha1', 'sha1_git', 'blake2s256'])
def test_skipped_content_add(self): def test_skipped_content_add(self):
cont = self.skipped_cont.copy() cont = self.skipped_cont.copy()
cont2 = self.skipped_cont2.copy() cont2 = self.skipped_cont2.copy()
cont2['blake2s256'] = None cont2['blake2s256'] = None
self.storage.content_add([cont, cont, cont2]) self.storage.content_add([cont, cont, cont2])
▲ Show 20 Lines • Show All 194 Lines • ▼ Show 20 Lines def test_revision_log(self):
self.storage.revision_add([self.revision3, self.storage.revision_add([self.revision3,
self.revision4]) self.revision4])
# when # when
actual_results = list(self.storage.revision_log( actual_results = list(self.storage.revision_log(
[self.revision4['id']])) [self.revision4['id']]))
# hack: ids generated # hack: ids generated
for actual_result in actual_results: for actual_result in actual_results:
if 'id' in actual_result['author']:
del actual_result['author']['id'] del actual_result['author']['id']
if 'id' in actual_result['committer']:
del actual_result['committer']['id'] del actual_result['committer']['id']
douarddaUnsubmitted
Not Done
Inline Actions

These hunks (in test_storage.py) should not be in this Diff. Having to do so in order to make your test_in_memory pass (I guess it's the reason for these modifications) prove your implementation does not respect the "API" of the original storage class.

It's probably no big deal here, but it's a bad practice...

douardda: These hunks (in test_storage.py) should not be in this Diff. Having to do so in order to make…
vlorentzAuthorUnsubmitted
Done
Inline Actions

comments like # hack: ids generated show that these IDs are quirks from the SQL backend, not features.

vlorentz: comments like `# hack: ids generated` show that these IDs are quirks from the SQL backend, not…
vlorentzAuthorUnsubmitted
Done
Inline Actions

Actually, according to Storage's docstrings, actual_result['author'] and actual_result['committer'] should not even exist.

vlorentz: Actually, according to Storage's docstrings, `actual_result['author']` and `actual_result…
self.assertEqual(len(actual_results), 2) # rev4 -child-> rev3 self.assertEqual(len(actual_results), 2) # rev4 -child-> rev3
self.assertEqual(actual_results[0], self.assertEqual(actual_results[0],
self.normalize_entity(self.revision4)) self.normalize_entity(self.revision4))
self.assertEqual(actual_results[1], self.assertEqual(actual_results[1],
self.normalize_entity(self.revision3)) self.normalize_entity(self.revision3))
def test_revision_log_with_limit(self): def test_revision_log_with_limit(self):
# given # given
# self.revision4 -is-child-of-> self.revision3 # self.revision4 -is-child-of-> self.revision3
self.storage.revision_add([self.revision3, self.storage.revision_add([self.revision3,
self.revision4]) self.revision4])
actual_results = list(self.storage.revision_log( actual_results = list(self.storage.revision_log(
[self.revision4['id']], 1)) [self.revision4['id']], 1))
# hack: ids generated # hack: ids generated
for actual_result in actual_results: for actual_result in actual_results:
if 'id' in actual_result['author']:
del actual_result['author']['id'] del actual_result['author']['id']
if 'id' in actual_result['committer']:
del actual_result['committer']['id'] del actual_result['committer']['id']
self.assertEqual(len(actual_results), 1) self.assertEqual(len(actual_results), 1)
self.assertEqual(actual_results[0], self.revision4) self.assertEqual(actual_results[0], self.revision4)
@staticmethod @staticmethod
def _short_revision(revision): def _short_revision(revision):
return [revision['id'], revision['parents']] return [revision['id'], revision['parents']]
Show All 27 Linesclass CommonTestStorage(TestStorageData):
def test_revision_get(self): def test_revision_get(self):
self.storage.revision_add([self.revision]) self.storage.revision_add([self.revision])
actual_revisions = list(self.storage.revision_get( actual_revisions = list(self.storage.revision_get(
[self.revision['id'], self.revision2['id']])) [self.revision['id'], self.revision2['id']]))
# when # when
if 'id' in actual_revisions[0]['author']:
del actual_revisions[0]['author']['id'] # hack: ids are generated del actual_revisions[0]['author']['id'] # hack: ids are generated
if 'id' in actual_revisions[0]['committer']:
del actual_revisions[0]['committer']['id'] del actual_revisions[0]['committer']['id']
self.assertEqual(len(actual_revisions), 2) self.assertEqual(len(actual_revisions), 2)
self.assertEqual(actual_revisions[0], self.assertEqual(actual_revisions[0],
self.normalize_entity(self.revision)) self.normalize_entity(self.revision))
self.assertIsNone(actual_revisions[1]) self.assertIsNone(actual_revisions[1])
def test_revision_get_no_parents(self): def test_revision_get_no_parents(self):
self.storage.revision_add([self.revision3]) self.storage.revision_add([self.revision3])
▲ Show 20 Lines • Show All 1,116 Lines • Show Last 20 Lines
About · Developer information · Legal