RafaAguilar · January 28, 2017 17:33
diff --git a/conceptual_test.py b/conceptual_test.py
 from db_model.models.content import RawArticle
 from db_model.models.source import Source
 from engine.celery import app, BaseTask
 from engine.resources.vimeo_service import fetch_user_videos
 from engine.resources.youtube_service import get_youtube_uploads
 from time import strptime
 from engine.utils.lock import FetchLock
 import requests
 '''Imports: 
    Mi primera observación sería ordenar los imports, conservando el orden
    según manda PEP8, primero los import core, seguidos de una linea en blanco
    luego los módulos de tereceros y por último los módules locales, además
    de separar cada import por línea e.g:
        from engine.celery import app
        from engine.celery import BaseTask
 '''
 # 2015-08-03T22:16:40+00:00
 date_format = '%Y-%m-%dT%H:%M:%S+00:00'
 dt2 = '%Y-%m-%dT%H:%M:%S+00:00'
 '''No ahorrar bytes, que ahora sobran, para los nombres de variables,
   es decir, si vamos a tener dos formatos (o más) distintos de formatos de
   fechas es mejor usar alguna estructura como un diccionario. Sin embargo veo
   que `dt2` no se usa más.
 '''
 def fetch_vimeo(source_id):
    source = Source.get_one_by_id(source_id)
    v = RawArticle.get_last_for_source_id(source.id) 
    # Usar nombre completo en variable, como: last_article, por ejemplo.
    accessToken = source.credential.data.get('access-token', None);
    videos = fetch_user_videos(source.external_id, accessToken)
    videos.reverse() # 
    raw_articles = []
    for video in videos:
        if v is None or strptime(video['created_time'], date_format) > \
                strptime(v.data['created_time'], date_format):
            raw_articles.append(RawArticle(data=video, source=source))
        '''Siempre es mejor tardar un par de segundos más escribiendo software
           para hacerlo más legible. e.g:
    # El ciclo no debería empezar si `v is None`
    # Tambien podemos aprovecharnos de el valor booleano de para verificar
    # si `v` es None (==Falsy)
    if v:
        last_article_date = strptime(v.data['created_time'], date_format
        # ^^^^^^ iría fuera del loop, para calcularlo sólo una vez
            for video in videos:
                new_artticle_date = strptime(video['created_time'], date_format)
                if new_artticle_date > base_article_date:
                    raw_articles.append(RawArticle(data=video, source=source))        
        '''
    '''
     En la siguiente línea no se verifica que `raw_articles` tenga registros
     antes de salvarlos, lo cual puede ser _misleading_ para el error que se
     levanta en el camino `else`
    '''
    if RawArticle.save_many(raw_articles):
        source.register_fetch_success()
        source.save()
    else:
        '''Se recomienda crear Excepciones particulares para identificar los 
         errores, además que el mensaje que se coloca no lleva información
         relevante
         e.g. RawArticleException('No articles were saved')
        '''
        source.register_fetch_error(Exception('something horrible happened'))
            source.save() # está mal identado.
 # Entre metodos fuera de las clases, debería haber dos líneas de por medio.
 def fetch_youtube(source_id):
    source = Source.get_one_by_id(source_id)
    last_article = RawArticle.get_last_for_source_id(source.id)
    last_client_id = last_article.client_id if last_article else None
    ''' Al igual que el metodo anterior, si `last_article` es  None, no deberia
        seguir la ejecución, debido a que el ID psoteriormente.
    '''
    articles = []
    api_key = source.credential.data.get("api_key", None)
    feed_data =  get_youtube_uploads(source.external_id, api_key)
    # el método `get_youtube_uploads` puede subir una excepción y esta no es 
    # capturada en este nivel.
    filtered_items = []
    for item in feed_data["items"]:
        if item["snippet"]["resourceId"]["videoId"] == last_client_id:
            break
        filtered_items.append(item)
    ras=[] # Usar un nombre completo y no abreviaciones.
    '''Debido a que filtered_items no se usa luego, se recomienda usar
       `filtered_items.reverse()` para no crear un duplicado en memoria
    '''
    for item in reversed(filtered_items):
        ras.append(RawArticle(data=item, source=source, client_id=item["snippet"]["resourceId"]["videoId"]))
        # ^^^^^la linea excede de 80 caracteres, se puede partir la linea por argumentos
    # IDEM al método anterior
    if RawArticle.save_many(ras):
        source.register_fetch_success()
        source.save()
    else:
        source.register_fetch_error(Exception('something horrible happened'))
 def get_youtube_uploads(channel_id, api_key):
    params = {
        "part": "id,snippet,contentDetails",
        "playlistId": channel_id,
        "key": api_key
    }
    # La linea excede de 80 carácteres.
    res = requests.get('https://www.googleapis.com/youtube/v3/playlistItems', params=params)
    if not res.ok:
        # Requests provee de excepciones que se podría usar para hacer más
        # explícito el error, en caso de no querer crear propias excepciones.
        raise (Exception(res.text))
        # Si lo unico que se necesita notificar es que no hay resultados, sería
        # mejor devolver un objeto vació, en este caso diccionario vacío.
    return res.json()
	from db_model.models.content import RawArticle
	from db_model.models.source import Source
	from engine.celery import app, BaseTask
	from engine.resources.vimeo_service import fetch_user_videos
	from engine.resources.youtube_service import get_youtube_uploads
	from time import strptime
	from engine.utils.lock import FetchLock
	import requests
	'''Imports:
	Mi primera observación sería ordenar los imports, conservando el orden
	según manda PEP8, primero los import core, seguidos de una linea en blanco
	luego los módulos de tereceros y por último los módules locales, además
	de separar cada import por línea e.g:
	from engine.celery import app
	from engine.celery import BaseTask
	'''
	# 2015-08-03T22:16:40+00:00
	date_format = '%Y-%m-%dT%H:%M:%S+00:00'
	dt2 = '%Y-%m-%dT%H:%M:%S+00:00'
	'''No ahorrar bytes, que ahora sobran, para los nombres de variables,
	es decir, si vamos a tener dos formatos (o más) distintos de formatos de
	fechas es mejor usar alguna estructura como un diccionario. Sin embargo veo
	que `dt2` no se usa más.
	'''
	def fetch_vimeo(source_id):
	source = Source.get_one_by_id(source_id)
	v = RawArticle.get_last_for_source_id(source.id)
	# Usar nombre completo en variable, como: last_article, por ejemplo.
	accessToken = source.credential.data.get('access-token', None);
	videos = fetch_user_videos(source.external_id, accessToken)
	videos.reverse() #
	raw_articles = []
	for video in videos:
	if v is None or strptime(video['created_time'], date_format) > \
	strptime(v.data['created_time'], date_format):
	raw_articles.append(RawArticle(data=video, source=source))
	'''Siempre es mejor tardar un par de segundos más escribiendo software
	para hacerlo más legible. e.g:
	# El ciclo no debería empezar si `v is None`
	# Tambien podemos aprovecharnos de el valor booleano de para verificar
	# si `v` es None (==Falsy)
	if v:
	last_article_date = strptime(v.data['created_time'], date_format
	# ^^^^^^ iría fuera del loop, para calcularlo sólo una vez
	for video in videos:
	new_artticle_date = strptime(video['created_time'], date_format)
	if new_artticle_date > base_article_date:
	raw_articles.append(RawArticle(data=video, source=source))
	'''
	'''
	En la siguiente línea no se verifica que `raw_articles` tenga registros
	antes de salvarlos, lo cual puede ser _misleading_ para el error que se
	levanta en el camino `else`
	'''
	if RawArticle.save_many(raw_articles):
	source.register_fetch_success()
	source.save()
	else:
	'''Se recomienda crear Excepciones particulares para identificar los
	errores, además que el mensaje que se coloca no lleva información
	relevante
	e.g. RawArticleException('No articles were saved')
	'''
	source.register_fetch_error(Exception('something horrible happened'))
	source.save() # está mal identado.
	# Entre metodos fuera de las clases, debería haber dos líneas de por medio.
	def fetch_youtube(source_id):
	source = Source.get_one_by_id(source_id)
	last_article = RawArticle.get_last_for_source_id(source.id)
	last_client_id = last_article.client_id if last_article else None
	''' Al igual que el metodo anterior, si `last_article` es None, no deberia
	seguir la ejecución, debido a que el ID psoteriormente.
	'''
	articles = []
	api_key = source.credential.data.get("api_key", None)
	feed_data = get_youtube_uploads(source.external_id, api_key)
	# el método `get_youtube_uploads` puede subir una excepción y esta no es
	# capturada en este nivel.
	filtered_items = []
	for item in feed_data["items"]:
	if item["snippet"]["resourceId"]["videoId"] == last_client_id:
	break
	filtered_items.append(item)
	ras=[] # Usar un nombre completo y no abreviaciones.
	'''Debido a que filtered_items no se usa luego, se recomienda usar
	`filtered_items.reverse()` para no crear un duplicado en memoria
	'''
	for item in reversed(filtered_items):
	ras.append(RawArticle(data=item, source=source, client_id=item["snippet"]["resourceId"]["videoId"]))
	# ^^^^^la linea excede de 80 caracteres, se puede partir la linea por argumentos
	# IDEM al método anterior
	if RawArticle.save_many(ras):
	source.register_fetch_success()
	source.save()
	else:
	source.register_fetch_error(Exception('something horrible happened'))
	def get_youtube_uploads(channel_id, api_key):
	params = {
	"part": "id,snippet,contentDetails",
	"playlistId": channel_id,
	"key": api_key
	}
	# La linea excede de 80 carácteres.
	res = requests.get('https://www.googleapis.com/youtube/v3/playlistItems', params=params)
	if not res.ok:
	# Requests provee de excepciones que se podría usar para hacer más
	# explícito el error, en caso de no querer crear propias excepciones.
	raise (Exception(res.text))
	# Si lo unico que se necesita notificar es que no hay resultados, sería
	# mejor devolver un objeto vació, en este caso diccionario vacío.
	return res.json()