Support google file api urls #2270
Conversation
| @@ -92,6 +92,9 @@ class FileUrl(ABC): | |||
| url: str | |||
| """The URL of the file.""" | |||
|
|
|||
| _media_type: str | None = field(default=None, repr=False) | |||
There was a problem hiding this comment.
@DouweM I diverged from your proposition of setting media_type in the __post_init__, because I realized (after the fact...) that it would make it mandatory to specify it in cases where it's not needed/relevant, e.g. when force_download==True or when it's a youtube video url. (I figured making the field optional would be too messy)
There was a problem hiding this comment.
Ok, defining it as a private field (with the underscore) makes sense, but since we want users to set this, the keyword argument on the constructor should not have an underscore. So I think we'll need a custom __init__ method (and overrides on the subclasses)
There was a problem hiding this comment.
I did it using an InitVar + __post_init__ to avoid having to define the whole init for FileUrl + subclasses. Is that ok?
There was a problem hiding this comment.
I commented below but the argument really should be media_type without the confusing final underscore. Since this is a library, the cleanness of the public interface is more important than the cleanness of the code to make it work. :)
| @@ -92,6 +92,9 @@ class FileUrl(ABC): | |||
| url: str | |||
| """The URL of the file.""" | |||
|
|
|||
| _media_type: str | None = field(default=None, repr=False) | |||
There was a problem hiding this comment.
Ok, defining it as a private field (with the underscore) makes sense, but since we want users to set this, the keyword argument on the constructor should not have an underscore. So I think we'll need a custom __init__ method (and overrides on the subclasses)
| document_url = DocumentUrl(url='https://example.com/document.pdf') | ||
| assert document_url.media_type == 'application/pdf' | ||
| assert document_url.format == 'pdf' | ||
|
|
||
| document_url = DocumentUrl(url='https://example.com/document', media_type_='application/pdf') |
There was a problem hiding this comment.
Can't we make the argument media_type? That may require a custom __init__.
Uh oh!
There was an error while loading. Please reload this page.