Frapell enhancements #398
Frapell enhancements #398
Conversation
| return ext.lower() == '.svg' and imgsize < 40960 | ||
| if ext.lower() == '.svg' and imgsize < 40960: | ||
| # Do not assume an image is an SVG based only in file extension | ||
| if os.path.exists(imgpath): |
There was a problem hiding this comment.
Hay casos donde preguntamos por la imagen y la misma no está?
There was a problem hiding this comment.
No debería, pero mimetype.from_file levanta una excepción si no encuentra el archivo y tenés que arrancar todo el proceso de nuevo... Me parece mas prudente dejar que termine y después uno decide si la imagen que faltó (y que no debería) amerita empezar la corrida nuevamente.
There was a problem hiding this comment.
Te cambio el punto de vista de la pregunta.
Esa imagen siempre está, no vale la pena preguntar si existe. Si esa imagen no está, realmente tenemos un bug en otro lado (fijate de donde se llama a esta función). Entonces, no preguntes si la imagen está, porque estás escondiendo un potencial problema.
| @@ -130,11 +130,16 @@ def fetch_html(url): | |||
| try: | |||
| req = urllib.request.Request(url, headers=REQUEST_HEADERS) | |||
| resp = urllib.request.urlopen(req, timeout=60) | |||
| compressedstream = io.BytesIO(resp.read()) | |||
| resp_content = resp.read() | |||
| compressedstream = io.BytesIO(resp_content) | |||
There was a problem hiding this comment.
¿este cambio para qué es?
There was a problem hiding this comment.
Es porque corté el branch de antes que se mergee #397 suponia que luego de mergearse no iba a mostrar el diff, no se por que lo sigue mostrando...
There was a problem hiding this comment.
En tu máquina local, contra master, no ves esta parte del diff?
There was a problem hiding this comment.
Fijate que este PR dice que está "outdated contra master", quizás te falta actualizar tu branch, y por eso se sigue viendo acá?
| new_split_name = list() | ||
| for i in range(int(len(name)/limit)+1): | ||
| new_part = name[i*limit:(i+1)*limit] | ||
| new_part.replace('.', '').replace('-', '').replace('/', '') |
There was a problem hiding this comment.
No entiendo el por qué de estos replaces...
There was a problem hiding this comment.
El - no recuerdo por qué está ahí... pero el . y el / son por las dudas que puedieran generar algun conflicto en algún sistema de archivos, pero los puedo sacar, si te parece.
There was a problem hiding this comment.
No agreguemos código "por las dudas". Si no podés crear un test en test_extract.py que "explote" eso, entonces no hay un caso que amerite...
|
Ahí contesté las conversaciones, gracias!! |
@fzuccolo @facundobatista Acá van las mejoras que les comente en #397
Longitud maxima de archivo
Note que habia muchos errores de
OSError: [Errno 36] File name too longpor ejemplo con https://upload.wikimedia.org/wikipedia/commons/thumb/a/ab/%D0%9E%D1%81%D0%BD%D0%BE%D0%B2%D0%BD%D0%B8%D0%B9_%D0%BC%D0%BE%D1%82%D0%B8%D0%B2_%D0%BF%D0%B5%D1%87%D0%B0%D1%82%D0%BE%D0%BA_%D0%92%D1%96%D0%B9%D1%81%D1%8C%D0%BA%D0%B0_%D0%97%D0%B0%D0%BF%D0%BE%D1%80%D0%BE%D0%B6%D1%81%D1%8C%D0%BA%D0%BE%D0%B3%D0%BE_%D0%BD%D0%B8%D0%B7%D0%BE%D0%B2%D0%BE%D0%B3%D0%BE.png/30px-%D0%9E%D1%81%D0%BD%D0%BE%D0%B2%D0%BD%D0%B8%D0%B9_%D0%BC%D0%BE%D1%82%D0%B8%D0%B2_%D0%BF%D0%B5%D1%87%D0%B0%D1%82%D0%BE%D0%BA_%D0%92%D1%96%D0%B9%D1%81%D1%8C%D0%BA%D0%B0_%D0%97%D0%B0%D0%BF%D0%BE%D1%80%D0%BE%D0%B6%D1%81%D1%8C%D0%BA%D0%BE%D0%B3%D0%BE_%D0%BD%D0%B8%D0%B7%D0%BE%D0%B2%D0%BE%D0%B3%D0%BE.png que aparece en este articulo https://es.wikipedia.org/wiki/Organizaci%C3%B3n_territorial_de_UcraniaLo que hago es agarrar el nombre completo, y splitearlo en X subcarpetas de N longitud, donde N es un tamaño que se setea en el
config.pyactualmente en 240 caracteresComo resultado, el archivo local es llamado
/images/commons/thumb/a/ab/30px-%25D0%259E%25D1%2581%25D0%25BD%25D0%25BE%25D0%25B2%25D0%25BD%25D0%25B8%25D0%25B9_%25D0%25BC%25D0%25BE%25D1%2582%25D0%25B8%25D0%25B2_%25D0%25BF%25D0%25B5%25D1%2587%25D0%25B0%25D1%2582%25D0%25BE%25D0%25BA_%25D0%2592%25D1%2596%25D0%25B9%25D1%2581%25D1%258C%25D0%25BA%25D0%25B0_%25D0%2597%25D0%25B0%25D0%25BF%25D0%25BE%25D1%2580%25D0%25BE%25D0%25B6%25D1%2581%25D1%258C%25D0%25BA%25D0%25BE%25D0/%25B3%25D0%25BE_%25D0%25BD%25D0%25B8%25D0%25B7%25D0%25BE%25D0%25B2%25D0%25BE%25D0%25B3%25D0%25BE.png(hay una barra metida ahi al medio :P) de manera que el archivo puede ser incluido.No basarse unicamente en extension para saber si es SVG
En este articulo https://es.wikipedia.org/wiki/Artur_Chiling%C3%A1rov la firma tiene extension SVG, cuando en realidad es un PNG. Por este unico archivo, la ejecucion explotaba cuando intentaba hacer el embed. Lo que hice fue incluir
python-magiccomo dependencia y a la hora de encontrar un archivo con extension SVG, realmente fijarse si es un SVG o no.