Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix download without path arguments#1591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

GabrielSimonetto
Copy link
Contributor

@Poolitzer
Copy link
Member

Poolitzer commentedOct 31, 2019•
edited by Bibo-Joshi
Loading

thanks for this.

After a short discussion, we came to the conclusion that you are actually on something here. Telegram does define file_name, on which file_path is based, as optional. If you want to include this, we ask to change this though:

  • Make a proper check if file_path exists
  • If not, set the file_path to the string of the file_id
  • implement a unitest for this case

This way, the actual filename will be unique. We dont think that this is ever going to happen, but better be safe then sorry

@GabrielSimonetto
Copy link
ContributorAuthor

Ok, it will be done soon enough

@GabrielSimonetto
Copy link
ContributorAuthor

So... I have a problem.

I don't have experience with tests, I've read some of them ontests/test_file.py, but I don't understand how they work. So I would need to do a test that build aFIle with the constructor usingfile_id, and then download and see if an error ocurs, but on this module we don't have a valid file_id to do this, so I don't know how that could be done. Can someone point me in some direction?

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commentedNov 18, 2019•
edited
Loading

Hi, sorry for the late reply. Intest_file.py, a fixturefile is defined (Line 29). This acts just like an ordinary Telegram File. You can pass it to your new test like

def test_new_test(self, bot, file):    file.file_id = 5                # Just an example    assert file.file_id == 5

Within that test you can change the attributes offile as needed for your test. If you have more questions, please don't hesitate to ask :)

Also, please merge from master so we can make sure all the other tests are passing, too.

@Bibo-JoshiBibo-Joshi added 📋 pending-replywork status: pending-reply bug 🐛 labelsNov 24, 2019
@Bibo-JoshiBibo-Joshi added this to the12.5 milestoneNov 24, 2019
@Bibo-Joshi
Copy link
Member

@GabrielSimonetto I took the liberty to go ahead and implement the test case ;)

@GabrielSimonetto
Copy link
ContributorAuthor

Thanks@Bibo-Joshi, sorry for disappearing, I've been a bit busy this month.

@Bibo-JoshiBibo-Joshi modified the milestones:12.5,12.4Feb 2, 2020
Copy link
Member

@jh0kerjh0ker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM 👍

@Eldinnie
Copy link
Member

Thanks you for your contribution

@EldinnieEldinnie merged commitbacabbe intopython-telegram-bot:masterFeb 6, 2020
Eldinnie added a commit that referenced this pull requestFeb 6, 2020
* Rename Test suite* Actually change the badge in readme* fix download without path arguments (#1591)* fix download without path arguments* fix download without path arguments* solved downloading a file without file_path or custom_path* if no file_path, download as file_id* Add test case* Elaborate doc stringCo-authored-by: Bibo-Joshi <hinrich.mahler@freenet.de>* Add default values (#1490)* added parse_mode parameter in Updater and in Bot class to set a default parse mode for bot* DefaultValue* Add default parse_mode everywhere* Renome to default_parse_mode* Test def parse_mode for send_*, edit_message_** Remove duplicate code in Input* classes* Improve handling of def_p_m for Bot methods* Remove unneeded deletion of kwargs* Make@log preserve signature, add bots with defaults to tests* Fix Codacy* Fix Travis Error* Add default_disable_notification* Add default_disable_web_page_preview* Add default_disable_timeout* add default_disable_web_page_preview for InputTextMessageContent* add default_quote* Try fixing test_pin_and_unpin* Just run 3.5 tests for paranoia* add tests for Defaults* Revert "Just run 3.5 tests for paranoia"This reverts commit1baa91a.* Tidy up parameters, move Defaults to ext* Stage new files, because im an idiot* remove debug prints* change equality checks for DEFAULT_NONE* Some last changes* fix S&R error so that tests actually runCo-authored-by: Ak4zh <agwl.akash@gmail.com>Co-authored-by: Eldinnie <Eldinnie@users.noreply.github.com>* Skip test relying on ordered dicts for 3.5 (#1752)* Rename Test suite* Actually change the badge in readmeCo-authored-by: Gabriel Simonetto <42247511+GabrielSimonetto@users.noreply.github.com>Co-authored-by: Ak4zh <agwl.akash@gmail.com>Co-authored-by: Eldinnie <Eldinnie@users.noreply.github.com>
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsAug 19, 2020
@Bibo-JoshiBibo-Joshi added 🔌 bugpr description: bug and removed bug 🐛 labelsNov 3, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@jh0kerjh0kerjh0ker approved these changes

Assignees
No one assigned
Labels
🔌 bugpr description: bug📋 pending-replywork status: pending-reply
Projects
None yet
Milestone
12.4
Development

Successfully merging this pull request may close these issues.

[BUG] Cannot Download a File if it doesnt have afile_path or ifcustom_path is not provided todownload()
5 participants
@GabrielSimonetto@Poolitzer@Bibo-Joshi@Eldinnie@jh0ker
[8]ページ先頭
©2009-2025 Movatter.jp