Page MenuHomeSoftware Heritage
Differential D4829

Add mapping of sha1 with swh ID
ClosedPublic
Actions

Authored by TG1999 on Jan 8 2021, 5:28 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDMFCD6d70494b713e: Add mapping of sha1 with SWH ID
Summary

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Diff Detail

Repository
rDMFCD ClearlyDefined metadata fetcher
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
TG1999 added a comment.Jan 9 2021, 6:20 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 9 2021, 6:20 AM

Build has FAILED

Patch application report for D4829 (id=17118)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit 7cb20067fc6438834fe29b1f08a1b2591a7e388d
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 9 10:49:54 2021 +0530

    Add mapping of sha1 with swh ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 9 2021, 6:20 AM
Harbormaster failed remote builds in B18260: Diff 17118!
TG1999 updated this revision to Diff 17119.Jan 9 2021, 11:41 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 9 2021, 11:41 AM

Build has FAILED

Patch application report for D4829 (id=17119)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit a4c4853361f59da81ac8ccd920e00b7a05b10cd8
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 9 16:10:11 2021 +0530

    Add mapping of sha1 with swh ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 9 2021, 11:41 AM
Harbormaster failed remote builds in B18261: Diff 17119!
TG1999 updated this revision to Diff 17207.Jan 14 2021, 11:53 AM
  • Ignore mypy for psycopg2
swh-public-ci added a comment.Jan 14 2021, 11:54 AM

Build is green

Patch application report for D4829 (id=17207)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit 530f4bcab595994b2ea939d379f47c4f23fa1d62
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Thu Jan 14 16:22:10 2021 +0530

    Ignore mypy for psycopg2
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

commit a4c4853361f59da81ac8ccd920e00b7a05b10cd8
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 9 16:10:11 2021 +0530

    Add mapping of sha1 with swh ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster completed remote builds in B18351: Diff 17207.Jan 14 2021, 11:54 AM
TG1999 requested review of this revision.Jan 14 2021, 11:54 AM
vlorentz requested changes to this revision.Jan 14 2021, 12:14 PM
vlorentz added a subscriber: vlorentz.

(marking this diff as "changes requested" because it needs tests, as discussed on IRC)

This revision now requires changes to proceed.Jan 14 2021, 12:14 PM
TG1999 updated this revision to Diff 17282.Jan 16 2021, 7:29 AM
  • Add tests for sha1 mapping with SWHID
swh-public-ci added a comment.Jan 16 2021, 7:30 AM

Build has FAILED

Patch application report for D4829 (id=17282)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit f4d53ae1b7be721365b3a28f36606454449a5cf4
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 16 11:57:59 2021 +0530

    Add tests for sha1 mapping with SWHID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

commit 530f4bcab595994b2ea939d379f47c4f23fa1d62
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Thu Jan 14 16:22:10 2021 +0530

    Ignore mypy for psycopg2
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

commit a4c4853361f59da81ac8ccd920e00b7a05b10cd8
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 9 16:10:11 2021 +0530

    Add mapping of sha1 with swh ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster failed remote builds in B18422: Diff 17282!Jan 16 2021, 7:30 AM
TG1999 updated this revision to Diff 17284.Jan 16 2021, 7:36 AM

Add tests for mapping of sha1 with SWHID

swh-public-ci added a comment.Jan 16 2021, 7:36 AM

Build has FAILED

Patch application report for D4829 (id=17284)

Could not rebase; Attempt merge onto 31111a6fe2...

Updating 31111a6..7689644
Fast-forward
 conftest.py                              |  1 +
 mypy.ini                                 |  3 +++
 requirements-swh.txt                     |  1 +
 requirements.txt                         | 15 +++++++++++----
 swh/clearlydefined/mapping_utils.py      | 23 +++++++++++++++++++++++
 swh/clearlydefined/tests/test_mapping.py | 22 ++++++++++++++++++++++
 6 files changed, 61 insertions(+), 4 deletions(-)
 create mode 100644 conftest.py
 create mode 100644 swh/clearlydefined/mapping_utils.py
 create mode 100644 swh/clearlydefined/tests/test_mapping.py
Changes applied before test
commit 7689644b85d367f365f446a61be3a0d99361573f
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 16 12:04:21 2021 +0530

    Add tests for mapping of sha1 with SWHID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

commit 530f4bcab595994b2ea939d379f47c4f23fa1d62
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Thu Jan 14 16:22:10 2021 +0530

    Ignore mypy for psycopg2
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

commit a4c4853361f59da81ac8ccd920e00b7a05b10cd8
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 9 16:10:11 2021 +0530

    Add mapping of sha1 with swh ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster failed remote builds in B18423: Diff 17284!Jan 16 2021, 7:36 AM
TG1999 updated this revision to Diff 17285.Jan 16 2021, 7:39 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 16 2021, 7:40 AM

Build has FAILED

Patch application report for D4829 (id=17285)

Could not rebase; Attempt merge onto 31111a6fe2...

Updating 31111a6..51b3f61
Fast-forward
 conftest.py                              |  1 +
 mypy.ini                                 |  3 +++
 requirements-swh.txt                     |  1 +
 requirements.txt                         | 15 +++++++++++----
 swh/clearlydefined/mapping_utils.py      | 23 +++++++++++++++++++++++
 swh/clearlydefined/tests/test_mapping.py | 22 ++++++++++++++++++++++
 6 files changed, 61 insertions(+), 4 deletions(-)
 create mode 100644 conftest.py
 create mode 100644 swh/clearlydefined/mapping_utils.py
 create mode 100644 swh/clearlydefined/tests/test_mapping.py
Changes applied before test
commit 51b3f6165a6e6a543eac1a4f0d57af9ccd8d3dac
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 16 12:08:35 2021 +0530

    Add mapping of sha1 with SWHID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

commit a4c4853361f59da81ac8ccd920e00b7a05b10cd8
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 9 16:10:11 2021 +0530

    Add mapping of sha1 with swh ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster failed remote builds in B18424: Diff 17285!Jan 16 2021, 7:40 AM
TG1999 updated this revision to Diff 17286.Jan 16 2021, 7:46 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 16 2021, 7:47 AM

Build has FAILED

Patch application report for D4829 (id=17286)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit e0d98a3df14ccf3b1c0ce68f556087bd5bf791b5
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 16 12:15:47 2021 +0530

    Add mapping sha1 with SWHID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster failed remote builds in B18425: Diff 17286!Jan 16 2021, 7:47 AM
TG1999 updated this revision to Diff 17287.Jan 16 2021, 7:54 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 16 2021, 7:54 AM

Build has FAILED

Patch application report for D4829 (id=17287)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit f76960b144759c278ebcc8b3a834728a6e3e4ca4
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 16 12:23:27 2021 +0530

    Add mapping sha1 with SWHID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster failed remote builds in B18426: Diff 17287!Jan 16 2021, 7:54 AM
TG1999 updated this revision to Diff 17288.Jan 16 2021, 7:57 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 16 2021, 7:57 AM

Build has FAILED

Patch application report for D4829 (id=17288)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit 57e1a0658b2a26773b7a2d80f77bd1f497503551
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 16 12:26:34 2021 +0530

    Add mapping sha1 with SWHID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster failed remote builds in B18427: Diff 17288!Jan 16 2021, 7:57 AM
TG1999 updated this revision to Diff 17289.Jan 16 2021, 8:00 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 16 2021, 8:00 AM

Build is green

Patch application report for D4829 (id=17289)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit dbc33fc6f3816fd13a870934cbd213e8d7bce4b7
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Sat Jan 16 12:29:34 2021 +0530

    Add mapping sha1 with SWHID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster completed remote builds in B18428: Diff 17289.Jan 16 2021, 8:00 AM
vlorentz requested changes to this revision.Jan 16 2021, 10:52 AM

Thanks

Some things to improve:

  • Phabricator colors the column on the right side of the diff as orange or blue; orange means the line is not covered by tests, so you should add tests that trigger these lines
  • If I replace this line: cur.execute("SELECT sha1_git FROM content where sha1= %s;", (sha1,)) with this: cur.execute("SELECT sha1_git FROM content") this would add a bug, but tests wouldn't catch it.
  • Add type annotations and a docstring
  • Use bytes to represent hashes passed to postgresql. For example sha1 = hash_to_bytes(sha1) instead of sha1 = "\\x{sha1}".format(sha1=sha1). Your way works (which I just learned by reading it), but we already use bytes throughout the codebase, so it's better for consistency. It also prevents small mistakes in the future as it's a different type.
  • Move line breaks to be in a logical separation, instead of wherever the 89th column happened to be (eg. not in the middle of the value of blake2s256_byte)
  • Improve variable names to reflect what they actually contain (eg. sha1_byte is a string and not bytes, sha1_git = cur.fetchall() assignes a tuple of tuple not directly a sha1, etc.)
  • Fix the date in the existing copyright notice, and add one in the tests
  • Remove unrelated changes to requirements.txt (I'm not sure what they are for)
This revision now requires changes to proceed.Jan 16 2021, 10:52 AM
TG1999 updated this revision to Diff 17291.Jan 18 2021, 6:53 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 18 2021, 6:54 AM

Build has FAILED

Patch application report for D4829 (id=17291)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit 6d9393940c491beaa5208fc41faf422083d3c1ec
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Mon Jan 18 11:22:59 2021 +0530

    Add mapping of sha1 with SWH ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster failed remote builds in B18430: Diff 17291!Jan 18 2021, 6:54 AM
TG1999 updated this revision to Diff 17292.Jan 18 2021, 7:00 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 18 2021, 7:00 AM

Build has FAILED

Patch application report for D4829 (id=17292)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit 3a837fd9d75e3e810dcb861dba77f28e93eae818
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Mon Jan 18 11:29:35 2021 +0530

    Add mapping of sha1 with SWH ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster failed remote builds in B18431: Diff 17292!Jan 18 2021, 7:00 AM
TG1999 updated this revision to Diff 17293.Jan 18 2021, 7:06 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 18 2021, 7:07 AM

Build is green

Patch application report for D4829 (id=17293)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit 55ef20716d4dfcf4073d6d257858982704cbe269
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Mon Jan 18 11:36:10 2021 +0530

    Add mapping of sha1 with SWH ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster completed remote builds in B18432: Diff 17293.Jan 18 2021, 7:07 AM
TG1999 updated this revision to Diff 17330.Jan 19 2021, 12:04 PM

Add mapping of sha1 with swh ID

TG1999 updated this revision to Diff 17332.Jan 19 2021, 12:05 PM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 19 2021, 12:05 PM

Build is green

Patch application report for D4829 (id=17330)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit e5448a0a69b3c82b77d067e33f6f050e01ce9510
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Tue Jan 19 16:33:54 2021 +0530

    Add mapping of sha1 with SWH ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster completed remote builds in B18469: Diff 17330.Jan 19 2021, 12:05 PM
swh-public-ci added a comment.Jan 19 2021, 12:06 PM

Build is green

Patch application report for D4829 (id=17332)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit a5b104f37b7fc55b84d039a54d5edf4da7319483
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Tue Jan 19 16:34:45 2021 +0530

    Add mapping of sha1 with SWH ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster completed remote builds in B18471: Diff 17332.Jan 19 2021, 12:06 PM
vlorentz requested changes to this revision.Jan 19 2021, 12:21 PM
vlorentz added inline comments.
swh/clearlydefined/tests/test_mapping.py
13–36

I was about to ask you to use hash_to_bytes here as well, but on second thoughts, it's more readable the way you did it, so let's keep it that way :)

61–65

This test doesn't check anything:

  1. if the function returns None like it should, then it does assert True and the test succeeds (as it should)
  1. if the function returns anything else, then the assert True doesn't run, and the test succeeds (but it shouldn't)

Could you update the test to fix that?

68–73

same issue here

This revision now requires changes to proceed.Jan 19 2021, 12:21 PM
TG1999 updated this revision to Diff 17368.Jan 20 2021, 7:10 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 20 2021, 7:11 AM

Build is green

Patch application report for D4829 (id=17368)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit 6c2e0a8915c7e3a410e59c4d77725d289c0ff92e
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Wed Jan 20 11:40:00 2021 +0530

    Add mapping of sha1 with SWH ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster completed remote builds in B18504: Diff 17368.Jan 20 2021, 7:11 AM
TG1999 updated this revision to Diff 17369.Jan 20 2021, 10:03 AM

Add mapping of sha1 with swh ID

vlorentz accepted this revision.Jan 20 2021, 10:04 AM
This revision is now accepted and ready to land.Jan 20 2021, 10:04 AM
swh-public-ci added a comment.Jan 20 2021, 10:04 AM

Build is green

Patch application report for D4829 (id=17369)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit 3de47ec8a425252e07d4096e89488fb3623593ed
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Wed Jan 20 14:33:08 2021 +0530

    Add mapping of sha1 with SWH ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster completed remote builds in B18505: Diff 17369.Jan 20 2021, 10:04 AM
vlorentz requested changes to this revision.Jan 20 2021, 10:05 AM
vlorentz added inline comments.
swh/clearlydefined/tests/test_mapping.py
11–12

one last thing: this connection is not used for reading

This revision now requires changes to proceed.Jan 20 2021, 10:05 AM
TG1999 updated this revision to Diff 17371.Jan 20 2021, 10:12 AM

Add mapping of sha1 with swh ID

swh-public-ci added a comment.Jan 20 2021, 10:13 AM

Build is green

Patch application report for D4829 (id=17371)

Rebasing onto 31111a6fe2...

Current branch diff-target is up to date.
Changes applied before test
commit 6d70494b713e7708f2cbe2294e9cc128bc88be3b
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Wed Jan 20 14:41:56 2021 +0530

    Add mapping of sha1 with SWH ID
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster completed remote builds in B18507: Diff 17371.Jan 20 2021, 10:13 AM
vlorentz accepted this revision.Jan 20 2021, 10:20 AM
This revision is now accepted and ready to land.Jan 20 2021, 10:20 AM
Closed by commit rDMFCD6d70494b713e: Add mapping of sha1 with SWH ID (authored by TG1999). · Explain WhyJan 20 2021, 10:21 AM
This revision was automatically updated to reflect the committed changes.
TG1999 added a commit: rDMFCD6d70494b713e: Add mapping of sha1 with SWH ID.

Revision Contents
Changeset List

PathSize
1 line
3 lines
2 lines
4 lines
swh/
clearlydefined/
29 lines
tests/
72 lines

Diff 17372

View Options

conftest.py

Loading...
View Options

mypy.ini

Loading...
View Options

requirements-swh.txt

Loading...
View Options

requirements.txt

Loading...
View Options

swh/clearlydefined/mapping_utils.py

Loading...
View Options

swh/clearlydefined/tests/test_mapping.py

Loading...
View Options

swh/clearlydefined/tests/test_nothing.py

Loading...
About · Developer information · Legal