Page MenuHomeSoftware Heritage
Differential D896

Use python-magic instead of file_magic.
ClosedPublic
Actions

Authored by vlorentz on Jan 8 2019, 2:00 PM.

Details

Reviewers
None
Group Reviewers
Reviewers
Maniphest Tasks
T861: mimetype indexer: edge case makes the indexer fail miserably
Commits
rDCIDXa6ce5993e8e4: 'import magic' then check it's the right one, instead of using pkg_resources.
rDCIDXa8668c5cb5e3: Add support for python-magic in addition to file_magic, and make it the default.
Summary

Depends on D895
Depends on T1504
Resolves T861

Diff Detail

Repository
rDCIDX Metadata indexer
Branch
python-magic
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8151
Build 11747: tox-on-jenkinsJenkins
Build 11746: arc lint + arc unit

Event Timeline

vlorentz created this revision.Jan 8 2019, 2:00 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJan 8 2019, 2:00 PM
Harbormaster failed remote builds in B3281: Diff 2831!Jan 8 2019, 2:01 PM
swh-public-ci added a subscriber: swh-public-ci.Jan 8 2019, 2:01 PM

Build has FAILED

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

vlorentz updated this revision to Diff 2832.Jan 8 2019, 2:17 PM
  • rebase
swh-public-ci added a comment.Jan 8 2019, 2:20 PM

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

Harbormaster completed remote builds in B3283: Diff 2832.Jan 8 2019, 2:20 PM
ardumont added a subscriber: ardumont.Jan 8 2019, 2:20 PM
ardumont added inline comments.
swh/indexer/mimetype.py
34

With this, the tool registration mechanism must change.
Right now, the tool used is declared in the configuration, thus static.

With this, that becomes dynamic.
To stay consistent, that must be recorded alongside the index result.

Right now, the configuration is the one by default referencing python3-magic [1]

[1] https://forge.softwareheritage.org/source/swh-indexer/browse/master/swh/indexer/mimetype.py$38-44

vlorentz planned changes to this revision.Jan 8 2019, 2:27 PM
vlorentz marked an inline comment as done.
vlorentz added inline comments.
swh/indexer/mimetype.py
34

Agreed. Let's put this diff on hold while we migrate to introspecting tools.

anlambert added a subscriber: anlambert.Jan 8 2019, 2:28 PM
anlambert added inline comments.
requirements.txt
3

I would keep file_magic by default here as python-magic is not packaged in Debian stretch (python3-magic deb package corresponds to file_magic).
Otherwise, a backport package for python-magic must be created for successfull deployment to production environment

vlorentz marked an inline comment as done.Jan 8 2019, 2:29 PM
vlorentz added inline comments.
requirements.txt
3

The problem is that the python-magic in Stretch has a bug that affects us (T861).

anlambert added inline comments.Jan 8 2019, 2:34 PM
requirements.txt
3

Might be related to a bug I also observed in the webapp, I used a dirty workaround that seems to do the job (see https://forge.softwareheritage.org/source/swh-web/browse/master/swh/web/browse/utils.py$71).

vlorentz marked an inline comment as done.Jan 8 2019, 2:36 PM
vlorentz added inline comments.
requirements.txt
3

I don't thing so, the bug T861 always happens for a given content.

anlambert added inline comments.Jan 8 2019, 2:41 PM
requirements.txt
3

Effectively, so backporting the python3-magic deb package from testing to stretch is required before production deployment. I could also benefit from it in the webapp and remove my dirty hack.

vlorentz updated this revision to Diff 2842.Jan 9 2019, 3:17 PM
  • Always use python-magic, drop file_magic.
vlorentz retitled this revision from Add support for python-magic in addition to file_magic, and make it the default. to Use python-magic instead of file_magic..Jan 9 2019, 3:18 PM
vlorentz edited the summary of this revision. (Show Details)
vlorentz added a task: T861: mimetype indexer: edge case makes the indexer fail miserably.
swh-public-ci added a comment.Jan 9 2019, 3:21 PM

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

Harbormaster completed remote builds in B3300: Diff 2842.Jan 9 2019, 3:21 PM
vlorentz updated this revision to Diff 2843.Jan 9 2019, 3:26 PM
  • Fix version.
swh-public-ci added a comment.Jan 9 2019, 3:29 PM

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

Harbormaster completed remote builds in B3301: Diff 2843.Jan 9 2019, 3:29 PM
olasd added a subscriber: olasd.Jan 9 2019, 5:31 PM
olasd added inline comments.
swh/indexer/mimetype.py
15–20

If they're both available with import magic, they're incompatible and shouldn't really be installed at the same time.

Could we just check that the magic module has an attribute that's only available in the version we want, and fail importing if it doesn't?

vlorentz updated this revision to Diff 2864.Jan 9 2019, 5:43 PM
vlorentz marked an inline comment as done.
  • 'import magic' then check it's the right one, instead of using pkg_resources.
swh-public-ci added a comment.Jan 9 2019, 5:46 PM

Build has FAILED

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

Harbormaster failed remote builds in B3325: Diff 2864!Jan 9 2019, 5:46 PM
swh-public-ci added a comment.Jan 9 2019, 5:51 PM

Build has FAILED

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

vlorentz added a comment.Jan 9 2019, 6:13 PM

(the build failure is caused by D897)

swh-public-ci added a comment.Jan 10 2019, 11:13 AM

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

swh-public-ci added a comment.Jan 10 2019, 11:49 AM

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

vlorentz updated this revision to Diff 2877.Jan 10 2019, 11:50 AM
  • rebuild
swh-public-ci added a comment.Jan 10 2019, 11:54 AM

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

Harbormaster completed remote builds in B3350: Diff 2877.Jan 10 2019, 11:54 AM
ardumont added a comment.EditedJan 11 2019, 12:13 PM

I'm ok with the idea.

This needs to be in pending state or something.
This diff's code depends on a not-yet existing backport package python-magic to be built IIUC.

ardumont added a comment.Feb 11 2019, 4:31 PM

Depends on T1504

ardumont edited the summary of this revision. (Show Details)Feb 11 2019, 4:32 PM
ardumont edited the summary of this revision. (Show Details)
vlorentz planned changes to this revision.Jun 26 2019, 12:05 PM
vlorentz updated this revision to Diff 6984.Oct 7 2019, 4:26 PM
  • rebase
  • squash some commits
swh-public-ci added a comment.Oct 7 2019, 4:28 PM

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

Harbormaster completed remote builds in B8151: Diff 6984.Oct 7 2019, 4:28 PM
This revision was not accepted when it landed; it landed in state Needs Review.Oct 7 2019, 4:29 PM
Closed by commit rDCIDXa8668c5cb5e3: Add support for python-magic in addition to file_magic, and make it the default. (authored by vlorentz). · Explain Why
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDCIDXa8668c5cb5e3: Add support for python-magic in addition to file_magic, and make it the default..
vlorentz added a commit: rDCIDXa6ce5993e8e4: 'import magic' then check it's the right one, instead of using pkg_resources..
vlorentz mentioned this in T861: mimetype indexer: edge case makes the indexer fail miserably.Nov 22 2019, 10:54 AM

Revision Contents
Changeset List

PathSize
2 lines
swh/
indexer/
31 lines

Diff 6984

View Options

requirements.txt

Loading...
View Options

swh/indexer/mimetype.py

Loading...
About · Developer information · Legal