Make use of content-type to detect format #252

pierredittgen · 2018-10-10T13:03:42Z

Currently, when using tabulator with URL sources, format is infered from:

suffix (e.g. https://example.com/files/foobar.csv -> csv)
format URL parameter (e.g. https://example.com/foobar?format=xls&param=baz -> xls)

One step beyond: when an URL has no format suffix, neither format url parameter, tabulator can request HEAD information and infer format from Content-type information. E.g content type 'text/csv' means csv format.

Content-type to format is defined as a dict (CONTENT_TYPE_FORMAT) that can be extended as needed to support more mime-types.

akariv · 2018-10-15T08:10:31Z

tabulator/helpers.py

@@ -17,6 +18,19 @@

 # Module API

+# Maps mime-type to format


Why not use https://docs.python.org/3.6/library/mimetypes.html ?

akariv · 2018-10-15T08:12:07Z

tabulator/helpers.py

+            req = requests.head(source, allow_redirects=True)
+            if req.status_code == requests.codes.ok:
+                content_type = req.headers.get('Content-type')
+                if not content_type is None:


if content_type is not None
is more readable I think

akariv · 2018-10-15T08:12:24Z

tabulator/helpers.py

+            if req.status_code == requests.codes.ok:
+                content_type = req.headers.get('Content-type')
+                if not content_type is None:
+                    format = CONTENT_TYPE_FORMAT.get(content_type)


here you can use mimetypes.guess_extension(...)

akariv · 2018-10-15T08:13:56Z

tabulator/helpers.py

@@ -58,6 +72,14 @@ def detect_scheme_and_format(source):
        if query_string_format is not None and len(query_string_format) == 1:
            format = query_string_format[0]

+        # Test if format info can be extracted from Content-type header
+        elif source.startswith('http'):
+            req = requests.head(source, allow_redirects=True)


some exception handling is needed here.

akariv · 2018-10-15T08:14:44Z

@pierredittgen looks good, I added some comments.
Can you please make sure that tests pass after this change?

roll · 2018-10-15T08:19:38Z

@akariv
@pierredittgen
I have some input on this one.

I think we had this implemented already at some point in time (should be possible to find the implementation in the history). But I disabled it because it was really inaccurate (the main problem was with Github Mime-Types as I can remember).

Probably we need a flag to make this feature optional (disabled by default). Or something like this. If my remembering is correct.

akariv · 2018-10-15T08:26:15Z

I think that Github serves all raw files as text to avoid abuse by 3rd parties (as well as using them as a hosting service for images and videos). 'Regular' files from the internet should have more accurate MIME types I think.

…

On Mon, Oct 15, 2018 at 11:19 AM roll ***@***.***> wrote: @akariv <https://github.com/akariv> @pierredittgen <https://github.com/pierredittgen> I have some input on this one. I think we had this implemented already at some point in time (should be possible to find the implementation in the history). But I disabled it because it was really inaccurate (the main problem was with Github Mime-Types as I can remember). Probably we need a flag like to make this feature optional (disabled by default). Or something like this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#252 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAQMdUzkkp1LLPPU3XKQeutV5ql0hwL0ks5ulEUngaJpZM4XVSlE> .

akariv · 2018-10-15T08:27:06Z

But controlling this behaviour using a flag is not a bad idea.

…

On Mon, Oct 15, 2018 at 11:26 AM Adam Kariv ***@***.***> wrote: I think that Github serves all raw files as text to avoid abuse by 3rd parties (as well as using them as a hosting service for images and videos). 'Regular' files from the internet should have more accurate MIME types I think. On Mon, Oct 15, 2018 at 11:19 AM roll ***@***.***> wrote: > @akariv <https://github.com/akariv> > @pierredittgen <https://github.com/pierredittgen> > I have some input on this one. > > I think we had this implemented already at some point in time (should be > possible to find the implementation in the history). But I disabled it > because it was really inaccurate (the main problem was with Github > Mime-Types as I can remember). > > Probably we need a flag like to make this feature optional (disabled by > default). Or something like this. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#252 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAQMdUzkkp1LLPPU3XKQeutV5ql0hwL0ks5ulEUngaJpZM4XVSlE> > . >

cbenz · 2018-11-15T10:41:31Z

Has this pull-request been reviewed and accepted? What could we do @pierredittgen and I to move on?

The parameter use_http_content_type=False was added in latest commits.

roll · 2018-11-26T16:19:59Z

@cbenz
@pierredittgen
Sorry I've got out of the loop a little bit.

Happy to review but not sure what's status of the PR (the flag, tests).

johanricher · 2019-03-21T09:38:10Z

What is missing to merge this PR?

roll · 2019-03-25T15:48:52Z

Sorry for the really late reply. I'm probably slightly out of sync and can be wrong.

But I have a few questions:

so we add an additional http call to the helper function which TBH wasn't intended to be not pure. I was wondering we need to check the http header inside the Remote Loader during the same (as data collection) call. But I guess it requires some design changes (I have to check)
not sure how as a user I can set the use_http_content_type flag. Will not it be always False?
we need tests, readme's API update

pierredittgen · 2019-03-27T08:09:12Z

not sure how as a user I can set the use_http_content_type flag. Will not it be always False?

In fact, in our code, we directly call the detect_scheme_and_format function to try to get the format of the given resource. Then, we can pass this info to the "inspect" method of goodtables.inspector.

stale · 2019-07-15T07:31:04Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2019-10-27T14:40:49Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

johanricher · 2019-10-28T11:45:56Z

@roll It seems a lot of work & progress was made here but not totally finished. What's left to do in order to merge this PR? On our side, @pierredittgen can still be available to help close this. :)

roll · 2019-10-28T15:14:51Z

@johanricher

The main problem was that it'd been intended to update the helpers.detect_scheme_and_format function which is:

not a part of public API
wasn't created to contain any IO calls

We need to add a publically available option to the remote loader:

https://github.com/frictionlessdata/tabulator-py#httphttpsftpftps

And probably copy this code to something like helpers.get_format_by_content_type and use it inside the Stream.open (we can't use it inside the remote loader because a loader can't update a file format...). TBH not sure exactly but something like this.

stale · 2020-01-26T15:52:36Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

akariv reviewed Oct 15, 2018

View reviewed changes

Pierre Dittgen and others added 4 commits October 16, 2018 14:44

Make use of content-type to detect format

74e630d

Follow review comments

577f1b2

Merge branch 'master' into master

14b038c

Fix linter warning

9f390fa

pierredittgen and others added 5 commits February 7, 2019 08:37

Get ints and datetimes from .xls files (#255)

a5cc036

Fix csv inline errors regarding compression (#257)

00c029b

v1.18.0

156e923

Extract ints, dates and datetimes from ods spreadsheet (#258)

77463bc

v1.19.0

df428be

roll added this to Software in Frictionless General Mar 19, 2019

roll moved this from Software to In progress in Frictionless General Mar 19, 2019

roll moved this from Pull Requests to Software in Frictionless General Mar 25, 2019

Merge branch 'master' into master

736f874

roll added the wip label Apr 15, 2019

stale bot added the wontfix label Jul 15, 2019

Merge branch 'master' into master

cecfdbf

stale bot removed the wontfix label Jul 29, 2019

roll added review and removed wip labels Sep 3, 2019

stale bot added the wontfix label Oct 27, 2019

stale bot removed the wontfix label Oct 28, 2019

stale bot added the wontfix label Jan 26, 2020

stale bot closed this Feb 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make use of content-type to detect format #252

Make use of content-type to detect format #252

pierredittgen commented Oct 10, 2018

akariv Oct 15, 2018

akariv Oct 15, 2018

akariv Oct 15, 2018

akariv Oct 15, 2018

akariv commented Oct 15, 2018

roll commented Oct 15, 2018 •

edited

akariv commented Oct 15, 2018 via email

akariv commented Oct 15, 2018 via email

cbenz commented Nov 15, 2018

roll commented Nov 26, 2018

johanricher commented Mar 21, 2019

roll commented Mar 25, 2019 •

edited

pierredittgen commented Mar 27, 2019

stale bot commented Jul 15, 2019

stale bot commented Oct 27, 2019

johanricher commented Oct 28, 2019

roll commented Oct 28, 2019 •

edited

stale bot commented Jan 26, 2020

Make use of content-type to detect format #252

Make use of content-type to detect format #252

Conversation

pierredittgen commented Oct 10, 2018

akariv Oct 15, 2018

Choose a reason for hiding this comment

akariv Oct 15, 2018

Choose a reason for hiding this comment

akariv Oct 15, 2018

Choose a reason for hiding this comment

akariv Oct 15, 2018

Choose a reason for hiding this comment

akariv commented Oct 15, 2018

roll commented Oct 15, 2018 • edited

akariv commented Oct 15, 2018 via email

akariv commented Oct 15, 2018 via email

cbenz commented Nov 15, 2018

roll commented Nov 26, 2018

johanricher commented Mar 21, 2019

roll commented Mar 25, 2019 • edited

pierredittgen commented Mar 27, 2019

stale bot commented Jul 15, 2019

stale bot commented Oct 27, 2019

johanricher commented Oct 28, 2019

roll commented Oct 28, 2019 • edited

stale bot commented Jan 26, 2020

roll commented Oct 15, 2018 •

edited

roll commented Mar 25, 2019 •

edited

roll commented Oct 28, 2019 •

edited