code/twython
1
Fork 0
Code Issues 23 Pull requests 4 Projects Releases 5 Packages Wiki Activity Actions

added media_category and support for STATUS calls #428

Merged
tushdante merged 4 commits from tushdante-video-upload into master 2016-09-30 04:28:38 -07:00
Conversation 8 Commits 4 Files changed 1 +36 -3
tushdante commented 2016-09-14 15:39:41 -07:00 (Migrated from github.com)
Copy link
  • added an additional media_category param value for upload_video()
  • added support for the STATUS command, when uploading videos asynchronously
- added an additional `media_category` param value for `upload_video()` - added support for the `STATUS` command, when uploading videos asynchronously
coveralls commented 2016-09-14 15:41:50 -07:00 (Migrated from github.com)
Copy link

Coverage Status

Coverage decreased (-0.8%) to 55.248% when pulling 2c02b622a2 on tushdante:tushdante-video-upload into 975095d8d2 on ryanmcgrath:master.

[![Coverage Status](https://coveralls.io/builds/7892084/badge)](https://coveralls.io/builds/7892084) Coverage decreased (-0.8%) to 55.248% when pulling **2c02b622a2cf8db5a2f060b3cb5672ff4e88d189 on tushdante:tushdante-video-upload** into **975095d8d20cb9966d0226b32a70b7a41cef2fb4 on ryanmcgrath:master**.
michaelhelmick (Migrated from github.com) requested changes 2016-09-23 06:23:49 -07:00
twython/endpoints.py
michaelhelmick (Migrated from github.com) commented 2016-09-23 06:20:52 -07:00
Copy link

Can you change these double quotes to single quotes, please?

Can you change these double quotes to single quotes, please?
michaelhelmick (Migrated from github.com) commented 2016-09-23 06:21:22 -07:00
Copy link

You don't need to default to None as check_after_secs = response_finalize['processing_info'].get('check_after_secs') will return None if it doesn't exist

You don't need to default to `None` as `check_after_secs = response_finalize['processing_info'].get('check_after_secs')` will return `None` if it doesn't exist
michaelhelmick (Migrated from github.com) commented 2016-09-23 06:21:37 -07:00
Copy link

You don't need to default to None as processing_state = response_finalize['processing_info'].get('state') will return None if it doesn't exist

You don't need to default to `None` as `processing_state = response_finalize['processing_info'].get('state')` will return `None` if it doesn't exist
twython/endpoints.py
@ -203,0 +226,4 @@
# get the secs to wait
check_after_secs = response.get('processing_info').get('check_after_secs')
if check_after_secs:
michaelhelmick (Migrated from github.com) commented 2016-09-23 06:23:26 -07:00
Copy link

Instead of changing the return here, can you add a keyword argument to upload_video like check_progress=False and if they pass check_progress=True, it will execute the code you added, otherwise it will just return the response.

Instead of changing the return here, can you add a keyword argument to `upload_video` like `check_progress=False` and if they pass `check_progress=True`, it will execute the code you added, otherwise it will just return the response.
coveralls commented 2016-09-25 18:08:03 -07:00 (Migrated from github.com)
Copy link

Coverage Status

Coverage decreased (-0.5%) to 55.54% when pulling 7401adfb64 on tushdante:tushdante-video-upload into 975095d8d2 on ryanmcgrath:master.

[![Coverage Status](https://coveralls.io/builds/8043239/badge)](https://coveralls.io/builds/8043239) Coverage decreased (-0.5%) to 55.54% when pulling **7401adfb640e50021cab7db2041f13a21ce6bad2 on tushdante:tushdante-video-upload** into **975095d8d20cb9966d0226b32a70b7a41cef2fb4 on ryanmcgrath:master**.
michaelhelmick (Migrated from github.com) requested changes 2016-09-26 05:36:09 -07:00
michaelhelmick (Migrated from github.com) left a comment
Copy link

Thanks for making the changes so quickly! Hopefully, we can get these changes done and approved and this merged in!

Thanks for making the changes so quickly! Hopefully, we can get these changes done and approved and this merged in!
twython/endpoints.py
michaelhelmick (Migrated from github.com) commented 2016-09-26 05:32:34 -07:00
Copy link

Can you just make this if check_progress -- sorry, a lot of the review is just styling issues!

Can you just make this `if check_progress` -- sorry, a lot of the review is just styling issues!
michaelhelmick (Migrated from github.com) commented 2016-09-26 05:35:28 -07:00
Copy link

Can you make this:

response = self.post(upload_url, params=params)

outside of this if check_progress:

and use response everywhere you use response_finalize

and outside of the check, you can just return response then, does that make sense or should I elaborate more?

Can you make this: ``` python response = self.post(upload_url, params=params) ``` outside of this `if check_progress:` and use `response` everywhere you use `response_finalize` and outside of the check, you can just `return response` then, does that make sense or should I elaborate more?
tushdante (Migrated from github.com) reviewed 2016-09-26 12:43:49 -07:00
twython/endpoints.py
tushdante (Migrated from github.com) commented 2016-09-26 12:43:49 -07:00
Copy link

No worries! @michaelhelmick totally happy to ensure everything is compliant. appreciate the feedback!

No worries! @michaelhelmick totally happy to ensure everything is compliant. appreciate the feedback!
coveralls commented 2016-09-26 14:26:11 -07:00 (Migrated from github.com)
Copy link

Coverage Status

Coverage decreased (-0.7%) to 55.367% when pulling 469432bcf8 on tushdante:tushdante-video-upload into 975095d8d2 on ryanmcgrath:master.

[![Coverage Status](https://coveralls.io/builds/8059092/badge)](https://coveralls.io/builds/8059092) Coverage decreased (-0.7%) to 55.367% when pulling **469432bcf84793db1137aaa6c5b3a4f70f386ad9 on tushdante:tushdante-video-upload** into **975095d8d20cb9966d0226b32a70b7a41cef2fb4 on ryanmcgrath:master**.
tushdante commented 2016-09-28 15:01:08 -07:00 (Migrated from github.com)
Copy link

Hey @michaelhelmick just wanted to call out if this PR will be merge-able. Let me know if there's any other changes you need me to make

Hey @michaelhelmick just wanted to call out if this PR will be merge-able. Let me know if there's any other changes you need me to make
michaelhelmick (Migrated from github.com) approved these changes 2016-09-30 04:28:30 -07:00
michaelhelmick (Migrated from github.com) left a comment
Copy link

LGTM, thanks for the edits!

LGTM, thanks for the edits!
Sign in to join this conversation.
Reviewers
No reviewers
michaelhelmick
Labels
Clear labels   
Bug

   
Enhancement

   
Feature Suggestion
No labels
Bug
Enhancement
Feature Suggestion
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: code/twython#428
No description provided.
Delete branch "tushdante-video-upload"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?