Page MenuHomeSoftware Heritage

pg-storage: Adapt implem to write both origin visit updates and status
ClosedPublic

Authored by ardumont on Apr 30 2020, 12:03 PM.

Details

Summary

This partially reverts commit a720caed6eebbb68a9f9b5be554a52859aa052d6 (D2938).

That now (still) writes new origin visit status...

But, as before, prior to D2938:

  • update origin visit (with same values as origin visit status)
  • read from origin visit

Related to T2310#44043

That does not revert the new in-memory (D2937) nor the cassandra (D2939)
storage implementations.

Test Plan

tox is happy

-- Docs: https://docs.pytest.org/en/latest/warnings.html
===================================================================================== 912 passed, 23 skipped, 1 xfailed, 18 warnings in 249.27s (0:04:09) =====================================================================================
___________________________________________________________________________________________________________________ summary ___________________________________________________________________________________________________________________
  black: commands succeeded
  flake8: commands succeeded
  mypy: commands succeeded
  py3: commands succeeded
  congratulations :)

Diff Detail

Repository
rDSTO Storage manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Apr 30 2020, 12:03 PM

12:03:49 + git fetch -n https://forge.softwareheritage.org/source/staging.git +refs/tags/phabricator/diff/11023:diff-target +refs/tags/phabricator/base/11023:diff-base
12:03:49 fatal: Couldn't find remote ref refs/tags/phabricator/diff/11023

mmm, i must have something off locally.
I can't seem to open diff without those failing for invalid reasons...

...
oh i have actually logs that says as much skip staging unable to determine repository for this change
...

+ export 'EDITOR=/home/tony/bin/emacs -nw'
+ EDITOR='/home/tony/bin/emacs -nw'
+ /nix/store/zbjfiy8qwrnm440ln8i6xspsv0si3y56-arcanist-20200127/bin/arc diff origin/master
+ /nix/store/yks09vjp2p8k7w5xvg63qcs46jhcghny-emacs-26.3/bin/emacsclient --create-frame -nw /tmp/edit.8pznrsvn4io0o4cg/new-commit
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
 SKIP STAGING  Unable to determine repository for this change.
Created a new Differential revision:
        Revision URI: https://forge.softwareheritage.org/D3101

Included changes:
  M       swh/storage/db.py
  M       swh/storage/storage.py
ardumont edited the test plan for this revision. (Show Details)Apr 30 2020, 12:13 PM
ardumont updated this revision to Diff 11025.Apr 30 2020, 12:23 PM

Update tryout with a different editor to trigger

Did not change a thing to try it out with another editor:

EDITOR="nano" arc diff --update D3101
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
 SKIP STAGING  Unable to determine repository for this change.
Updated an existing Differential revision:
        Revision URI: https://forge.softwareheritage.org/D3101

Included changes:
  M       swh/storage/db.py
  M       swh/storage/storage.py
ardumont updated this revision to Diff 11035.Apr 30 2020, 1:01 PM

Adapted a .git/config parameter and try to check if that fixes it

ardumont updated this revision to Diff 11038.Apr 30 2020, 1:20 PM

Test 3

(out of a fresh new swh-storage clone)

Test 3
(out of a fresh new swh-storage clone)

And that worked...

~/bin/arc diff origin/master --update D3101
+ export 'EDITOR=/home/tony/bin/emacs -nw'
+ EDITOR='/home/tony/bin/emacs -nw'
+ /nix/store/zbjfiy8qwrnm440ln8i6xspsv0si3y56-arcanist-20200127/bin/arc diff origin/master --update D3101
+ /nix/store/yks09vjp2p8k7w5xvg63qcs46jhcghny-emacs-26.3/bin/emacsclient --create-frame -nw /tmp/edit.193ihvywc17o08wk/differential-update-comments
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
 PUSH STAGING  Pushing changes to staging area...
Enumerating objects: 15, done.
Counting objects: 100% (15/15), done.
Delta compression using up to 8 threads
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 2.87 KiB | 2.87 MiB/s, done.
Total 9 (delta 6), reused 1 (delta 0)
To forge.softwareheritage.org:/source/staging.git
 * [new tag]           2b95dd331bd946ba48e6f3693fb0b96a664032ac -> phabricator/base/11038
 * [new tag]           9f80a7ea9bec217fceb46a5a61ce9842f451bce2 -> phabricator/diff/11038
Updated an existing Differential revision:
        Revision URI: https://forge.softwareheritage.org/D3101

Included changes:
  M       swh/storage/db.py
  M       swh/storage/storage.py

¯\_(ツ)_/¯

the first line of the commit message should mention you also changed the reads

vlorentz accepted this revision.Apr 30 2020, 1:26 PM

other than that, lgtm

This revision is now accepted and ready to land.Apr 30 2020, 1:26 PM

Build is green

Patch application report for D3101 (id=11038)

Rebasing onto 2b95dd331b...

Current branch diff-target is up to date.
Changes applied before test
commit 9f80a7ea9bec217fceb46a5a61ce9842f451bce2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 30 13:20:08 2020 +0200

    pg-storage: Adapt implem to write both origin visit updates and status
    
    This partially reverts commit a720caed6eebbb68a9f9b5be554a52859aa052d6 (D2938).
    
    That now (still) writes new origin visit status...
    
    But, as before, prior to D2938:
    
    - update origin visit (with same values as origin visit status)
    - read from origin visit
    
    Related to T2310#44043
    
    That does not revert the new in-memory (D2937) nor the cassandra (D2939)
    storage implementations.

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

the first line of the commit message should mention you also changed the reads

indeed, it's not in the header but it's in the commit message...
i'll see if i can synthesize and make it enter within the header.

ardumont updated this revision to Diff 11039.Apr 30 2020, 1:38 PM

Rework commit message

Build is green

Patch application report for D3101 (id=11039)

Rebasing onto 2b95dd331b...

Current branch diff-target is up to date.
Changes applied before test
commit 1d7b886d1c373d7655926e823323847932e5eb81
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 30 13:20:08 2020 +0200

    pg: Write both origin visit updates & status, read from origin_visit
    
    This partially reverts commit [1].
    
    That now (still) writes new origin visit status...
    
    But, as before [1]:
    
    - update origin visit (with same values as origin visit status)
    - read from origin visit
    
    That does not revert the new in-memory (D2937) nor the cassandra (D2939)
    storage implementations.
    
    [1] a720caed6eebbb68a9f9b5be554a52859aa052d6 D2938
    Related to D2938
    Related to T2310#44043

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

Build is green

Patch application report for D3101 (id=11044)

Rebasing onto 0e8234f854...

Current branch diff-target is up to date.
Changes applied before test
commit b0b767b91ca077a14368eaac1f98120261d7460c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 30 13:20:08 2020 +0200

    pg: Write both origin visit updates & status, read from origin_visit
    
    This partially reverts commit [1].
    
    That now (still) writes new origin visit status...
    
    But, as before [1]:
    
    - update origin visit (with same values as origin visit status)
    - read from origin visit
    
    That does not revert the new in-memory (D2937) nor the cassandra (D2939)
    storage implementations.
    
    [1] a720caed6eebbb68a9f9b5be554a52859aa052d6 D2938
    Related to D2938
    Related to T2310#44043

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