-
Notifications
You must be signed in to change notification settings - Fork 100
MAX_SIZE minor bug fix #99
base: master
Are you sure you want to change the base?
Changes from 3 commits
abe7caf
b40ec53
716c248
874af06
f2869c3
0b0e3c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,6 +85,9 @@ target/ | |
# pyenv | ||
.python-version | ||
|
||
#pycharm | ||
.idea | ||
|
||
# celery beat schedule file | ||
celerybeat-schedule | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,7 +80,8 @@ def _check_media(self, media): | |
""" | ||
Checks whether the given MessageMedia should be downloaded or not. | ||
""" | ||
if not media or not self.max_size: | ||
# It is needed to chek the size with the max_size defined in config file | ||
if not media or media.document.size > self.max_size: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you check at least twice the Telethon documentation to make sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will fix it, let you know the result There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are some points need consideration, Media has multiple types and it is not easy to get the size of each object, I mean it is depended to the Type and calling media.document.size will only work for document type and no other. I think we need to use polymorphism to do that. each type has to implement an interface for general attributes like size, so calling something like media.getsize() return the size of the file regardless of the type of it, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
More like some utils method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I found an easier way to do that, but not sure yet, so far I realized that just document types have size attribute, so when calling export_utils.get_media_type(media) returns "document" we can refer to media.document.size and compare it to the max_size, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also in utils there is a method named get_file_location(media):, So already it has been implemented, I will use it and after test I will commit the code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just add another function to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its done !, how can I pull the changes ? |
||
return False | ||
if not self.types: | ||
return True | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This bypasses the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it is if you check the config file, it says "Setting to "0" will not download any media." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, that's not what I meant. I mean, if there are no types (i.e. "all media is valid"), the maximum size is not checked at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's right, So the file size should be checked before this line! |
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This belongs in your global
.gitignore
, so please undo this change.