Paths

Table of Contentst

Differential D799

loader.git: Improve GitLoaderFromArchive
ClosedPublic
Actions

Authored by anlambert on Dec 11 2018, 1:54 PM.

Tags

None

Subscribers

Details

Reviewers

Group Reviewers

Commits

rDLDGfccc901a7d46: loader: Improve GitLoaderFromArchive

Summary

While working on improving swh-web tests, I noticed a couple of issues with
GitLoaderFromArchive. That diff implements the following:

guess project name from archive name instead of path
add support for tar.gz tarballs
handle more tarball layouts
simplify test

Diff Detail

Repository

rDLDG Git loader

Branch

git-loader-from-archive-improvements

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 2971
Build 3794: tox-on-jenkins	Jenkins
Build 3793: arc lint + arc unit

Event Timeline

anlambert created this revision.Dec 11 2018, 1:54 PM

anlambert added a reviewer: Reviewers.Dec 11 2018, 1:54 PM

Build is green
See https://jenkins.softwareheritage.org/job/DLDG/job/tox/59/ for more details.

Harbormaster completed remote builds in B2968: Diff 2544.Dec 11 2018, 1:55 PM

ardumont added inline comments.Dec 11 2018, 2:24 PM

swh/loader/git/tests/test_loader.py
128	Maybe rename to remove the zip notion?
135–136	Can you explicit the boolean value with its name? I think it's to inhibit the tarball uncompression step, not completely sure though.

ardumont added inline comments.Dec 11 2018, 2:31 PM

swh/loader/git/utils.py
47	Is that a production behavior or a test one?

anlambert marked 3 inline comments as done.Dec 11 2018, 2:42 PM

anlambert added inline comments.

swh/loader/git/tests/test_loader.py
128	proposal: BaseGitLoaderFromArchiveTest
135–136	sure!
swh/loader/git/utils.py
47	The purpose here is to handle tarballs with an unexpected layout. Ideally, when encoutering a tarball named `testrepo.tgz`, we expect the following layout in it: testrepo/ ├── .git │ ├── branches │ ├── COMMIT_EDITMSG │ ├── config │ ├── description │ ├── HEAD ... But some times we can have layout like this: . ├── .git │ ├── branches │ ├── COMMIT_EDITMSG │ ├── config │ ├── description │ ├── HEAD ... or like this: other-repo-name/ ├── .git │ ├── branches │ ├── COMMIT_EDITMSG │ ├── config │ ├── description │ ├── HEAD ... The code below handles those cases. I can also remove it and indicate the expected layout in the docstring.

Update: Address some ardumont comments

Build is green
See https://jenkins.softwareheritage.org/job/DLDG/job/tox/62/ for more details.

Harbormaster completed remote builds in B2969: Diff 2545.Dec 11 2018, 2:46 PM

ardumont accepted this revision.Dec 11 2018, 2:53 PM

ardumont added inline comments.

swh/loader/git/tests/test_loader.py
128	sounds good!
swh/loader/git/utils.py
47	Ideally, when encoutering a tarball named testrepo.tgz, we expect the following layout in it: Ah yes, i think it was for the gitorious/googlecode git archives ingestion... The code below handles those cases. I can also remove it and indicate the expected layout in the docstring. Or Keep your code adaptation and move your comment and your 3 samples on the docstring ;)

This revision is now accepted and ready to land.Dec 11 2018, 2:53 PM

Update loader docstring

Harbormaster completed remote builds in B2971: Diff 2547.Dec 11 2018, 3:13 PM

Build is green
See https://jenkins.softwareheritage.org/job/DLDG/job/tox/63/ for more details.

ardumont accepted this revision.Dec 11 2018, 3:29 PM

Closed by commit rDLDGfccc901a7d46: loader: Improve GitLoaderFromArchive (authored by anlambert). · Explain WhyDec 11 2018, 3:31 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

swh/

loader/

git/

45 lines

tests/

46 lines

17 lines

Diff 2547

swh/loader/git/loader.py

Loading...

swh/loader/git/tests/test_loader.py

Loading...

swh/loader/git/utils.py

Loading...