Details

Reviewers

Group Reviewers

Commits

rDLDTAR685b7a5b7e85: tar.loader: Use urllib.parse.urlparse to actually parse url
rDLDTARb79b89c02d47: tar.loader: Improve docstring
rDLDTAR228ec385299a: loader: Only explicit needed params & adapt docstring accordingly
rDLDTARcd09277fb2af: tar.loader: Improve class name and use alias for retro-compatibility
rDLDTAR7ce28b6ddf3c: tar.loader: Improve class docstrings
rDLDTAR84193fa870d6: tar.loader: Explicit the need for an implementation
rDLDTAR6a9ba4e9005e: tar.loader: Fix typo
rDLDTAR100281f01e65: tar.loader: Rename method with clearer name
rDLDTAR55587351acca: tar.loader: Fix typos
rDLDTARdd808d2ee75f: resources/producer: Remove dead configuration
rDLDTAR1c5224b54583: README: Update documentation and remove obsolete documentation
rDLDTARd75516bf76bb: loader.tar: Add back the legacy behavior (deposit depends on it)
rDLDTAR5e658ed6fb0b: tar.tasks: Improve docstring and test the task
rDLDTAR2f4f234834fa: tar.loader: Make the loader-tar able to download remote artifact

Summary

Also:

Keep the legacy behavior (use the revision and snapshot's branch provided by the caller, e.g. the deposit loader depends on this)
Increase code coverage (50 -> 93%)
Remove dir loader inheritance (nightmare to maintain). We still need the dir loader for some functions but the main issue is resolved).
Remove deprecated producer code (we use the scheduler now)

Related T1351
Related T1431
Depends on D794

Use sample:

>>> url = 'https://ftp.gnu.org/gnu/8sync/8sync-0.1.0.tar.gz'
>>> origin = {'url': url, 'type': 'tar'}
>>> visit_date = 'Tue, 3 May 2017 17:16:32 +0200'
>>> last_modified = '2016-04-22 16:35'
>>> import logging
>>> logging.basicConfig(level=logging.DEBUG)
>>>
>>> from swh.loader.tar.tasks import LoadTarRepository
>>> l = LoadTarRepository()
>>> l.run_task(origin=origin, visit_date=visit_date,
...            last_modified=last_modified)
DEBUG:swh.scheduler.task.LoadTarRepository:Creating tar origin for https://ftp.gnu.org/gnu/8sync/8sync-0.1.0.tar.gz
DEBUG:swh.scheduler.task.LoadTarRepository:Done creating tar origin for https://ftp.gnu.org/gnu/8sync/8sync-0.1.0.tar.gz
DEBUG:swh.scheduler.task.LoadTarRepository:Creating origin_visit for origin 3 at time Tue, 3 May 2017 17:16:32 +0200
DEBUG:swh.scheduler.task.LoadTarRepository:Done Creating origin_visit for origin 3 at time Tue, 3 May 2017 17:16:32 +0200
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): ftp.gnu.org:443
DEBUG:urllib3.connectionpool:https://ftp.gnu.org:443 "GET /gnu/8sync/8sync-0.1.0.tar.gz HTTP/1.1" 200 221837
INFO:swh.scheduler.task.LoadTarRepository:Uncompress /tmp/swh.loader.tar._nh29j5e-18075/8sync-0.1.0.tar.gz to /tmp/swh.loader.tar._nh29j5e-18075/swh.loader.tar-7wox5_rx
DEBUG:swh.scheduler.task.LoadTarRepository:Sending 28 contents
DEBUG:swh.scheduler.task.LoadTarRepository:Done sending 28 contents
DEBUG:swh.scheduler.task.LoadTarRepository:Sending 7 directories
DEBUG:swh.scheduler.task.LoadTarRepository:Done sending 7 directories
DEBUG:swh.scheduler.task.LoadTarRepository:Sending 1 revisions
DEBUG:swh.scheduler.task.LoadTarRepository:Done sending 1 revisions
DEBUG:swh.scheduler.task.LoadTarRepository:Updating origin_visit for origin 3 with status full
DEBUG:swh.scheduler.task.LoadTarRepository:Done updating origin_visit for origin 3 with status full
DEBUG:swh.scheduler.task.LoadTarRepository:Clean up /tmp/swh.loader.tar._nh29j5e-18075
{'status': 'eventful'}

Test Plan

tox

Diff Detail

Repository

rDLDTAR Tarball Loader

Branch

improve-loader-tar

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 2957
Build 3769: tox-on-jenkins	Jenkins
Build 3768: arc lint + arc unit

Event Timeline

ardumont created this revision.Dec 8 2018, 5:54 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptDec 8 2018, 5:54 PM

Harbormaster failed remote builds in B2946: Diff 2523!Dec 8 2018, 5:55 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLDTAR/job/tox/49/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDTAR/job/tox/49/console

ardumont edited the summary of this revision. (Show Details)Dec 8 2018, 5:56 PM

ardumont added a parent revision: D794: loader.tar: Add some tests and remove dead code.

Remove print statement T.T

Harbormaster failed remote builds in B2947: Diff 2524!Dec 8 2018, 5:58 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLDTAR/job/tox/50/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDTAR/job/tox/50/console

Fix d/control tab instead of spaces T.T

Harbormaster failed remote builds in B2948: Diff 2525!Dec 8 2018, 5:59 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLDTAR/job/tox/51/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDTAR/job/tox/51/console

Dependent diff landed so trigger the new build for this

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

Harbormaster completed remote builds in B2954: Diff 2528.Dec 8 2018, 6:09 PM

ardumont edited the summary of this revision. (Show Details)Dec 8 2018, 6:21 PM

tar.tasks: Improve docstring and test the task
README: Update documentation and remove obsolete documentation

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

Harbormaster completed remote builds in B2955: Diff 2529.Dec 8 2018, 6:45 PM

Remove wrong comment

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

Harbormaster completed remote builds in B2956: Diff 2530.Dec 8 2018, 7:06 PM

ardumont marked 3 inline comments as done.Dec 8 2018, 7:07 PM

ardumont added inline comments.

swh/loader/tar/loader.py
165	That will need to be provided by the caller (so the lister here).
swh/loader/tar/tests/test_build.py
48	Apparently my initial mtime was wrong, thus the slight change here. Other than that, i used the same date as before with another readable format.
52	I used the same date as before with another format.
swh/loader/tar/tests/test_loader.py
168	Same i use the same date as before but with another format.

Heads up, it's cool and all but:

the diff is too big so i'll split this in multiple ones.
i need to update the diff (done locally and tests ok \m/) to add back the legacy behavior as deposit depends on it.

ardumont planned changes to this revision.Dec 9 2018, 1:04 PM

loader.tar: Add back the legacy behavior (deposit depends on it)

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

Harbormaster completed remote builds in B2957: Diff 2531.Dec 9 2018, 1:06 PM

ardumont edited the summary of this revision. (Show Details)Dec 9 2018, 1:09 PM

ardumont edited the summary of this revision. (Show Details)Dec 9 2018, 1:17 PM

tar.loader: Fix typos
resources/producer: Remove dead configuration

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

Harbormaster completed remote builds in B2958: Diff 2532.Dec 9 2018, 1:26 PM

vlorentz accepted this revision.Dec 10 2018, 9:34 AM

vlorentz added a subscriber: vlorentz.

vlorentz added inline comments.

debian/rules
4	Why this change?
swh/loader/tar/loader.py
78	with `urllib.parse`
152	Why not `self.log.debug(...)`?
165	Maybe rename the method to `get_tarball_url_to_retrieve`, to make it obvious it returns the value (rather than setting an object attribute)
169	`raise NotImplementedError()` (or make the class abstract)
216	s/tarballs/tarball/
267	rename it to `LocalTarLoader`?

This revision is now accepted and ready to land.Dec 10 2018, 9:34 AM

ardumont marked 8 inline comments as done.Dec 10 2018, 10:44 AM

ardumont added inline comments.

debian/rules
4	The initial version is nose related. That new one is pytest ;)
swh/loader/tar/loader.py
78	Thank you, will look into it.
152	because i want the user to be warned it will mess up it's local environment for debug reasons. That matches other implementations btw (pypi, etc...)
165	Right
169	Right, i forgot!
216	indeed.
267	I wanted to avoid having to migrate the loader deposit. I could do as you did with aliasing if you want.

tar.loader: Rename method with clearer name
tar.loader: Explicit the need for an implementation
tar.loader: Fix typo
tar.loader: Improve class name and use alias for retro-compatibility
tar.loader: Improve class docstrings
loader: Only explicit needed params & adapt docstring accordingly
tar.loader: Improve docstring

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

Harbormaster completed remote builds in B2959: Diff 2533.Dec 10 2018, 10:46 AM

tar.loader: Use urllib.parse.urlparse to actually parse url

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

Harbormaster completed remote builds in B2960: Diff 2534.Dec 10 2018, 10:50 AM

Rebase

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

Harbormaster completed remote builds in B2961: Diff 2535.Dec 10 2018, 11:42 AM

Closed by commit rDLDTAR2f4f234834fa: tar.loader: Make the loader-tar able to download remote artifact (authored by ardumont). · Explain WhyDec 10 2018, 12:23 PM

This revision was automatically updated to reflect the committed changes.

ardumont mentioned this in T1676: Bug: the deposit metadata is not added to the revision while those are present.Apr 19 2019, 9:16 AM

tar.loader: Make the loader-tar able to download remote artifact
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 2531

README.md

debian/control

debian/rules

requirements.txt

setup.py

swh/loader/tar/.gitignore

swh/loader/tar/build.py

swh/loader/tar/file.py

swh/loader/tar/loader.py

swh/loader/tar/producer.py

swh/loader/tar/tasks.py

swh/loader/tar/tests/test_build.py

swh/loader/tar/tests/test_loader.py

swh/loader/tar/tests/test_tasks.py

tar.loader: Make the loader-tar able to download remote artifactClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 2531

README.md

debian/control

debian/rules

requirements.txt

setup.py

swh/loader/tar/.gitignore

swh/loader/tar/build.py

swh/loader/tar/file.py

swh/loader/tar/loader.py

swh/loader/tar/producer.py

swh/loader/tar/tasks.py

swh/loader/tar/tests/test_build.py

swh/loader/tar/tests/test_loader.py

swh/loader/tar/tests/test_tasks.py

tar.loader: Make the loader-tar able to download remote artifact
ClosedPublic
Actions

Revision Contents
Changeset List