#396 Custom Youtube API using Data API v3

[MadMub]

The is a re-implementation of the current Youtube uploader, but built in-house. The goals are:

1. Provide all features of previous uploader
2. Improve upload automation process
3. Be built modular, to allow flexibility and for more features to come later
4. Integrate into Freeseer's framework when desired

[dideler]

Regarding httplib2.HttpLib2Error, do you mean to retry for all exceptions raised by httplib2? Perhaps it's better to be more specific like how you were with the httplib exceptions.

[dideler]

Did a bit of review tonight, I'll continue it tomorrow.

[dideler]

Should the end user see this? Looks like it should be a logging call to me.

Another problem is that the message is only printed when no arguments are passed, but the framework is also initialized when arguments are passed.

[MadMub]

Sorry my intention is vague, the framework initialized message is really
just a placeholder for indicating the framework was initialized in API mode
as opposed to command line usage. Basically if arguments are passed it
knows that it is command line usage and will thus handle differently
(potentially) than with interactive use.
On Nov 20, 2013 7:43 PM, "Dennis Ideler" notifications@github.com wrote:

In src/freeseer/framework/youtube.py:

•        parser.add_argument("-c", "--client_secrets", type=unicode,
  

•                            help="Path to client secrets file", default=client_secrets)
  

•        parser.add_argument("-t", "--token", type=unicode,
  

•                            help="Path to OAuth2 token", default=oauth2_token)
  

•        parser.add_argument("-f", "--files", type=unicode,
  

•                            help="Path to file or filefolder for upload", default=self.default_video_directory)
  

•        parser.add_argument("-y", "--yes", help="Assume yes", action="store_true")
  

•        self.flags = parser.parse_args(argv[1:])
  

•        self.cmd_line = True
  

•    # Framework was initialized for interactive use
  

•    else:
  

•        self.flags = parser.parse_args()
  

•        self.cmd_line = False
  

•        print "Framework initialized"
  

Should the end user see this? Looks like it should be a logging call to me.

Another problem is that the message is only printed when no arguments are
passed, but the framework is also initialized when arguments are passed.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/402/files#r7812091
.

[zxiiro]

If your gonna put in placeholder messages, just make it the actual message you intend to put in, in the first place

Problem with placeholder stuff is your gonna forget to come back and fix it, so do it right the first time.

[dideler]

Some general comments:

• Docstrings are missing [1, 2]
• We should be using format() for strings [3]

[zxiiro]

As stated in another comment. I'd prefer if you changed your Frameworks file to be more of an API and less of an application. Split off the application part to a Frontend CLI part which prints data and is interactive based on what the API returns.

[zxiiro]

Unless you have to, you should prefer to alphabetically sort your imports.

[mtomwing]

Just log these errors where you catch them.

The logger also supports printf/sprintf style formatting IIRC:

log.info('A retriable HTTP error %d occured', e.resp.status)

[MadMub]

I would prefer save them to a variable then make a logging call to each and
still need to have some form of variable for the retry logic
On Nov 28, 2013 9:21 PM, "Michael Tom-Wing" notifications@github.com
wrote:

In src/freeseer/framework/youtube.py:

•    response = None
  

•    error = None
  

•    retry = 0
  

•    while response is None:
  

•        try:
  

•            log.info("Uploading %s" % video_file)
  

•            status, response = insert_request.next_chunk()
  

•            if 'id' in response:
  

•                return "video id: %s was successfully uploaded." % response['id']
  

•            else:
  

•                return "The upload failed with an unexpected response: %s" % response
  

•        except HttpError as e:
  

•            if e.resp.status in self.retriable_status_codes:
  

•                error = "A retriable HTTP error %d occurred:\n%s" % (e.resp.status, e.content)
  

Just log these errors where you catch them.

The logger also supports printf/sprintf style formatting IIRC:

log.info('A retriable HTTP error %d occured', e.resp.status)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/402/files#r7994237
.

[mtomwing]

You can use the else clause of a try block to set a flag indicating that there was no exception raised.

try:
    # code that can raise exception
except Exception:
    # handle that stuff
else:
    # this is only executed if try DOES NOT raise any exceptions

[mtomwing]

Another thing, please sort your import blocks. If you're using vim, it's as single as selecting the import lines and doing :sort.

Also, import blocks should be organized in the following way:

import os # standard stuff

import mutagen # third party stuff

from freeseer.settings import FreeseerConfig #stuff located within your module

[mtomwing]

This comment doesn't seem necessary. Also there should only be one new line between the last import and the first global variable.

[dideler]

Agreed.

[dideler]

You can get rid of comments like this that don't add any extra information than reading the code.

[zxiiro]

status?

[MadMub]

Exams sorry, IMO ready but I believe there are still some cosmetic changes.
I'll do one more facelift after today before asking to merge again.
On Dec 18, 2013 11:06 AM, "Thanh Ha" notifications@github.com wrote:

status?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/402#issuecomment-30854033
.

[zxiiro]

Merged, thanks for your contribution!