-
Notifications
You must be signed in to change notification settings - Fork 1
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Function name fix, using uuid1mc, time format fix, TaskRegistry fix #60
Conversation
instead of assigning them to a set
.travis.yml
Outdated
@@ -16,11 +16,11 @@ services: | |||
before_script: | |||
- mkdir imgee/static/test_uploads | |||
script: | |||
- nosetests --with-coverage | |||
- python runtests.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to a ./runtests.sh
. Copy that from hgapp's template. Basically:
#!/bin/sh
export FLASK_ENV="TESTING"
coverage run `which nosetests --with-timer`
coverage report
.travis.yml
Outdated
install: | ||
- sudo apt-get -qq install zlib1g-dev libfreetype6-dev liblcms1-dev libwebp-dev libjpeg-dev libpng-dev libfreetype6-dev libtiff4-dev librsvg2-dev ghostscript imagemagick pandoc | ||
- pip install -r requirements.txt | ||
- pip install nose coverage BeautifulSoup4 Pillow | ||
- pip install nose coverage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add these to a test_requirements.txt
and use pip install -r test_requirements.txt
imgee/storage.py
Outdated
@@ -29,17 +29,23 @@ def get_resized_image(img, size, is_thumbnail=False): | |||
""" | |||
registry = imgee.registry | |||
img_name = img.name | |||
|
|||
if img.mimetype == 'image/gif': | |||
# if the gif file is animated, no need to resize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually we do need to resize with animation, but if this is non-trivial, file a ticket for this to be addressed separately. (Preserving animation is higher priority than resizing.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed as #55.
imgee/storage.py
Outdated
|
||
if img.mimetype == 'image/gif': | ||
# if the gif file is animated, no need to resize | ||
src_path = download_from_s3(img.name + img.extn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does img.extn
include a period? If so, it seems a bit redundant to store a period in the database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does include the period. and the period is stored in db too. and .extn
is a function property https://github.com/hasgeek/imgee/blob/master/imgee/models/stored_file.py#L58
imgee/storage.py
Outdated
@@ -121,7 +125,7 @@ def save_on_s3(filename, remotename='', content_type='', bucket='', folder=''): | |||
headers = { | |||
'Cache-Control': 'max-age=31536000', # 60*60*24*365 | |||
'Content-Type': get_file_type(fp, filename), | |||
'Expires': datetime.now() + timedelta(days=365) | |||
'Expires': str(datetime.utcnow() + timedelta(days=365)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this output in the correct format for an Expires header?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does. Passing datetime object like it was before also worked. The boto library converted it to string itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, these are S3 headers, not HTTP headers. Makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Boto works with datetimes, I'll say just pass that in and don't bother about casting to string.
imgee/tasks.py
Outdated
|
||
def get_all_keys(self): | ||
return list(self.connection.smembers(self.key)) | ||
# >> KEYS imgee:registry:default:* | ||
return self.connection.keys(self._make_key("*")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single quotes
imgee/tasks.py
Outdated
|
||
def keys_starting_with(self, query): | ||
# >> KEYS imgee:registry:default:query* | ||
return self.connection.keys(self._make_key("{}*".format(query))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single quotes and query validation.
imgee/tasks.py
Outdated
def remove_keys_matching(self, query): | ||
keys = self.search(query) | ||
for k in keys: | ||
self.remove_by_key(k) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a Redis pipe and push all commands at once, unless there's a way to do a wildcard remove directly in Redis.
imgee/tasks.py
Outdated
def remove_keys_starting_with(self, query): | ||
keys = self.keys_starting_with(query) | ||
for k in keys: | ||
self.remove_by_key(k) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a pipe.
imgee/tasks.py
Outdated
|
||
def remove_all(self): | ||
for k in self.get_all_keys(): | ||
self.remove_by_key(k) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a pipe
@@ -216,7 +222,7 @@ def resize_and_save(img, size, is_thumbnail=False): | |||
Get the original image from local disk cache, download it from S3 if it misses. | |||
Resize the image and save resized image on S3 and size details in db. | |||
""" | |||
src_path = download_frm_s3(img.name + img.extn) | |||
src_path = download_from_s3(img.filename) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed this is for download. Since img.extn
is calculated, does this mean we're not actually saving the exact filename as stored in S3? That is a problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are storing the the filename as stored in S3. I'm not sure why this function exists https://github.com/hasgeek/imgee/blob/master/imgee/utils.py#L88
if the file name is abcd.png
, in db, we're storing name=abcd, orig_extn=.png
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I get it. We're not storing the exact name in s3 in db. orig_extn
is the original extension of the file. but as the thumbnail extension can be different from original extension, that target extension is stored in the ALLOWED_MIMETYPES
dict, and taken from there every time we request for the extension.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed as #69.
imgee/tasks.py
Outdated
|
||
def search(self, query): | ||
return filter(lambda k: str(query) in k, self.connection.smembers(self.key)) | ||
# >> KEYS imgee:registry:default:*query* | ||
if not re.compile(self.filename_pattern).match(query): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compilation is pointless at runtime. You'll get the exact same performance using re.match(self.filename_pattern, query)
. Move the regex compilation to the module level, and move the sanity check into an independent function so you can call it anywhere you're accepting a query.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, abort with a custom exception instead of returning list()
. This error should never occur as you're not passing in untrusted user input, so if it somehow does happen, you want it showing up in the server error log.
@@ -4,15 +4,16 @@ | |||
import os.path | |||
from subprocess import check_output, CalledProcessError | |||
from urlparse import urljoin | |||
from uuid import uuid4 | |||
|
|||
from boto import connect_s3 | |||
from boto.s3.bucket import Bucket | |||
from boto.s3.key import Key | |||
import defusedxml.cElementTree as elementtree | |||
from flask import request | |||
import magic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If magic
is a Python standard lib, it should be imported above, before third party libs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's from a 3rd party lib python-magic
. https://github.com/ahupp/python-magic
imgee/utils.py
Outdated
|
||
from coaster.utils import uuid1mc | ||
import imgee |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this import for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imgee.storage.get_resized_image()
was being used somewhere below. Fixing by just importing that function instead of the whole module.
@@ -29,17 +29,23 @@ def get_resized_image(img, size, is_thumbnail=False): | |||
""" | |||
registry = imgee.registry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a circular import. Can you possibly define the registry before other modules are imported, to avoid the circular import?
One way is to move the registry's __init__
code into a distinct init_app
method and call that separately after the app is configured (__init__
should also take an optional app
parameter and if present call init_app
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, an init_app()
function in the tasks.py
(where TaskRegistry
exists), and initialize the registry there and assign it to let's say app.registry
, and call it from imgee/__init__.py
so that it can be used from anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, an init_app method on the class. Not in the module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the convention in Flask is that if you want to add something to the app, add it under app.extensions
(a dictionary). We already have app.extensions['lastuser']
. You could do app.extensions['taskregistry']
. I will not recommend this however as the task registry is specific to Imgee and is not a shared extension. It's better being referenced as a direct Python import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More comments. :-)
imgee/__init__.py
Outdated
@@ -48,4 +44,6 @@ def error403(error): | |||
app.config['MEDIA_DOMAIN'] = app.config['MEDIA_DOMAIN'].split(':', 1)[1] | |||
mkdir_p(app.config['UPLOADED_FILES_DEST']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed this mkdir
call. The should be in manage.py
as say manage.py init
. It should not be a runtime check.
@@ -29,17 +29,23 @@ def get_resized_image(img, size, is_thumbnail=False): | |||
""" | |||
registry = imgee.registry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the convention in Flask is that if you want to add something to the app, add it under app.extensions
(a dictionary). We already have app.extensions['lastuser']
. You could do app.extensions['taskregistry']
. I will not recommend this however as the task registry is specific to Imgee and is not a shared extension. It's better being referenced as a direct Python import.
imgee/storage.py
Outdated
@@ -29,17 +29,23 @@ def get_resized_image(img, size, is_thumbnail=False): | |||
""" | |||
registry = imgee.registry | |||
img_name = img.name | |||
|
|||
if img.mimetype == 'image/gif': | |||
# if the gif file is animated, no need to resize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed as #55.
imgee/storage.py
Outdated
@@ -121,7 +125,7 @@ def save_on_s3(filename, remotename='', content_type='', bucket='', folder=''): | |||
headers = { | |||
'Cache-Control': 'max-age=31536000', # 60*60*24*365 | |||
'Content-Type': get_file_type(fp, filename), | |||
'Expires': datetime.now() + timedelta(days=365) | |||
'Expires': str(datetime.utcnow() + timedelta(days=365)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Boto works with datetimes, I'll say just pass that in and don't bother about casting to string.
@@ -216,7 +222,7 @@ def resize_and_save(img, size, is_thumbnail=False): | |||
Get the original image from local disk cache, download it from S3 if it misses. | |||
Resize the image and save resized image on S3 and size details in db. | |||
""" | |||
src_path = download_frm_s3(img.name + img.extn) | |||
src_path = download_from_s3(img.filename) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed as #69.
imgee/storage.py
Outdated
registry = imgee.registry | ||
|
||
# remove from registry | ||
registry.remove_keys_starting_with(stored_file.name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be …:<name>:*
? The lack of a separator is asking for future accidents.
imgee/storage.py
Outdated
|
||
# remove from registry | ||
registry.remove_keys_starting_with(stored_file.name) | ||
|
||
# remove locally | ||
cache_path = app.config.get('UPLOADED_FILES_DEST') | ||
cached_img_path = os.path.join(cache_path, '%s*' % stored_file.name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise here. Shouldn't there be an underscore separator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're good after these changes.
.travis.yml
Outdated
script: | ||
- nosetests --with-coverage | ||
- ./runtests.sh | ||
install: | ||
- sudo apt-get -qq install zlib1g-dev libfreetype6-dev liblcms1-dev libwebp-dev libjpeg-dev libpng-dev libfreetype6-dev libtiff4-dev librsvg2-dev ghostscript imagemagick pandoc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where have these dependencies gone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line has been removed. Don't we need these libraries anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Umm. It's not been removed. They are there.
Line 21 in e7f46fa
- sudo apt-get -qq install zlib1g-dev libfreetype6-dev liblcms1-dev libwebp-dev libjpeg-dev libpng-dev libfreetype6-dev libtiff4-dev librsvg2-dev ghostscript imagemagick pandoc |
imgee/__init__.py
Outdated
|
||
# Configure the app | ||
coaster.app.init_app(app) | ||
migrate = Migrate(app, db) | ||
baseframe.init_app(app, requires=['baseframe', 'picturefill', 'imgee']) | ||
lastuser.init_app(app) | ||
lastuser.init_usermanager(UserManager(db, models.User)) | ||
registry.init_app() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is supposed to get app
as a parameter. It shouldn't import the app.
imgee/__init__.py
Outdated
# using it to determine the project root | ||
# either get it from environment variable, or it's one level up from this init file | ||
app.project_root = os.environ.get('PYTHONPATH', '') or os.path.dirname(os.path.dirname(os.path.abspath(__file__))) | ||
print app.project_root |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove these two lines. They should be in the test script, not in code that runs in production.
imgee/__init__.py
Outdated
@@ -46,6 +47,3 @@ def error403(error): | |||
app.config['MEDIA_DOMAIN'].startswith('http:') or | |||
app.config['MEDIA_DOMAIN'].startswith('https:')): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace this if condition with if app.config.get('MEDIA_DOMAIN', '').lower().startswith(('http://', 'https://'))
imgee/tasks.py
Outdated
if app: | ||
self.init_app(name, connection) | ||
|
||
def init_app(self, name='default', connection=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
init_app
should not take any parameters apart from app
. The other parameters go directly to __init__
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set self.app = app
here.
imgee/utils.py
Outdated
@@ -84,7 +84,7 @@ | |||
|
|||
|
|||
def newid(): | |||
return unicode(uuid4().hex) | |||
return unicode(uuid1mc().hex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Back to uuid4
imgee/utils.py
Outdated
@@ -97,7 +97,7 @@ def get_file_url(scheme=None): | |||
|
|||
|
|||
def path_for(img_name): | |||
return os.path.join(app.config['UPLOADED_FILES_DEST'], img_name) | |||
return os.path.join(app.project_root, app.config['UPLOADED_FILES_DEST'], img_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't use this. If files are hosted under the static folder, use app.static_folder
manage.py
Outdated
|
||
@manager.command | ||
def init(): | ||
mkdir_p(os.path.join(app.project_root, app.config['UPLOADED_FILES_DEST'])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this command outside the if
clause.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but manager
is defined inside the if
block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add from coaster.manage import manager
at the top. init_manager
just returns the same manager. You can discard its return value and use the imported manager.
runtests.sh
Outdated
#!/bin/sh | ||
set -e | ||
export FLASK_ENV="TESTING" | ||
export PYTHONPATH=$(pwd) # setting project root as PYTHONPATH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use $(dirname $0)
here, since runtests.sh
could be called from another folder.
runtests.sh
Outdated
set -e | ||
export FLASK_ENV="TESTING" | ||
export PYTHONPATH=$(pwd) # setting project root as PYTHONPATH | ||
./manage.py init # creates the test upload directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this line to .travis.yml
, It's part of the install steps, not the test running steps.
- pip install -r requirements.txt | ||
- pip install nose coverage BeautifulSoup4 Pillow | ||
- pip install -r test_requirements.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a comment on the order of sections, to make this file more readable. It should be in install
, before_script
, script
order.
imgee/storage.py
Outdated
""" | ||
cache_path = app.config.get('UPLOADED_FILES_DEST') | ||
cache_path = os.path.join(app.static_folder, app.config.get('UPLOADED_FILES_DIR')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
os.path.join
will break out of app.static_folder
if the config setting has ..
or /
in it. I recall there's a secure version of path.join
that checks for this. Investigate it? (Not particularly serious though since it's only in server-side config.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check this out and make changes when I find it.
imgee/tasks.py
Outdated
self.key = 'imgee:registry:%s' % name | ||
self.connection = connection | ||
self.key_prefix = 'imgee:registry:%s' % name | ||
self.filename_pattern = '^[a-z0-9\_\.]+$' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to use re.compile(r'^[a-z0-9_\.]+$')
? It needs to be a raw r
string to remove ambiguity around the \
, and the .
needs escaping in regex, but not _
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
making it a raw string and not escaping _
. And we dont need to compile because as you mentioned before, no point compiling during runtime. passing the pattern directly to re.match()
.
imgee/tasks.py
Outdated
|
||
|
||
class TaskRegistry(object): | ||
def __init__(self, name='default', connection=None): | ||
self.connection = redis.from_url(connection) if connection else None | ||
def __init__(self, name='default', connection=None, app=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually app
is the first parameter after self
.
@@ -238,11 +248,12 @@ def get_no_previews_url(size): | |||
|
|||
|
|||
def get_image_url(image, size=None): | |||
from imgee import storage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's find a way to avoid importing inside functions. It should be the last resort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is a circular import issue. storage.py
imports a hell lot of stuff from this file but this file needs just one function from storage.py
. hence importing here, only where it's needed.
instance/settings.py
Outdated
@@ -34,6 +34,7 @@ | |||
LOGFILE = 'error.log' | |||
#: File uploads | |||
UPLOADED_FILES_DEST = 'imgee/static/uploads' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this config value still need to be present?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. forgot to delete this.
instance/settings.py
Outdated
@@ -34,6 +34,7 @@ | |||
LOGFILE = 'error.log' | |||
#: File uploads | |||
UPLOADED_FILES_DEST = 'imgee/static/uploads' | |||
UPLOADED_FILES_DIR = 'uploads' # this folder should be inside `imgee/static/` | |||
#: S3 Configuration | |||
AWS_ACCESS_KEY = 'Set aws access key here' | |||
AWS_SECRET_KEY = 'Set aws secret key here' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to encourage inserting values into a file that's version controlled in git. Maybe this file should be named settings-sample.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
settings.py
should be gitignored?
manage.py
Outdated
|
||
@manager.command | ||
def init(): | ||
mkdir_p(os.path.join(app.static_folder, app.config['UPLOADED_FILES_DIR'])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This os.path.join
call is in two places now. Best if it was in a function that's imported in both places.
tests/fixtures.py
Outdated
# PYTHONPATH is `pwd` when testing and empty otherwise | ||
# using it to determine the project root | ||
# either get it from environment variable, or it's one level up from this init file | ||
app.project_root = os.environ.get('PYTHONPATH', '') or os.path.dirname(os.path.dirname(os.path.abspath(__file__))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PYTHONPATH
is a path and can potentially contain multiple semicolon-separated entries. Best to use the dirname
method you have here and use that alone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're able to import imgee
, do a dirname
on the imgee
module instead of on __file__
. Also, why is this variable being added to the app
object? Where is it used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need this actually. removing it.
* removed celery as dependency * fixed - was missing folder name when downloading file * handling multiple resize request for same file * removed print statements * if there is an error while processing a file, raise it * fixed comment * removed all tests * removed more unnecessary things * removed unnecessary imports and minor cleanup * renamed async.py * added label tests * separating label logic * fixed label tests * more cleanup, moved some views out of index.py * cleaned up label saving codes a little * added tests for upload and delete * updated status code for delete test * mixed couple of migrations and showing error * some more cleanups * using get_image_url to create image url * not showing delete label button if not logged in * importing only the needed forms * added tests for resizing files and testing with multiple file types * comparing redirect location with generated thumbnail url * added redis to travis * fixed fixtures file to use new init_app behavior * Function name fix, using uuid1mc, time format fix, TaskRegistry fix (#60) * function name fix, using uuid1mc, time format fix * deleting keys from registry when deleting a file * restructured TaskRegistry to use Redis Keys with TTL set to 60 seconds instead of assigning them to a set * re-adding nose and coverage to travis file, removed it by mistake * using runtests.py file to run tests in travis * reordered functions and better string formating * made the redis key expiry time an argument * assigning error handler the right way * not resizing if the gif image is animated * added pillow as dependency * using redis pipeline to delete multiple keys * missed one place to use redis pipeline * deleted runtests.py, not needed anymore * fixed typo * fixed some import issues * moved upload directory creation to manage.py * added some comments * moved importing app inside the only function that uses it * Update runtests.sh * added manage.py init to runtests * removed mkdir from travis config * fixed upload path issue * moar fixes to registry, cleanup, moved back to uuid4 * fixed typo * setting test environment variable in travis file * setting env in existing block, otherwise it gets overwritten * moved the init command out of if block * rearranged travis config, cleanup of upload directory path * lot of minor fixes * using dotted relative import syntax everywhere, added tests for registry(redis) * cleaning up queries and imports * cleanup of os.path and more comments on thumbnail * fixed typo * added buildspec.yml file for testing AWS CodeBuild * setting env varialbe manually for aws codebuild * removed buildspec.yml file * showing loading spinner if file is still processing
* removed celery as dependency * fixed - was missing folder name when downloading file * handling multiple resize request for same file * removed print statements * if there is an error while processing a file, raise it * fixed comment * removed all tests * removed more unnecessary things * removed unnecessary imports and minor cleanup * renamed async.py * added label tests * separating label logic * fixed label tests * more cleanup, moved some views out of index.py * cleaned up label saving codes a little * added tests for upload and delete * updated status code for delete test * mixed couple of migrations and showing error * some more cleanups * using get_image_url to create image url * not showing delete label button if not logged in * importing only the needed forms * added tests for resizing files and testing with multiple file types * comparing redirect location with generated thumbnail url * added redis to travis * fixed fixtures file to use new init_app behavior * Function name fix, using uuid1mc, time format fix, TaskRegistry fix (#60) * function name fix, using uuid1mc, time format fix * deleting keys from registry when deleting a file * restructured TaskRegistry to use Redis Keys with TTL set to 60 seconds instead of assigning them to a set * re-adding nose and coverage to travis file, removed it by mistake * using runtests.py file to run tests in travis * reordered functions and better string formating * made the redis key expiry time an argument * assigning error handler the right way * not resizing if the gif image is animated * added pillow as dependency * using redis pipeline to delete multiple keys * missed one place to use redis pipeline * deleted runtests.py, not needed anymore * fixed typo * fixed some import issues * moved upload directory creation to manage.py * added some comments * moved importing app inside the only function that uses it * Update runtests.sh * added manage.py init to runtests * removed mkdir from travis config * fixed upload path issue * moar fixes to registry, cleanup, moved back to uuid4 * fixed typo * setting test environment variable in travis file * setting env in existing block, otherwise it gets overwritten * moved the init command out of if block * rearranged travis config, cleanup of upload directory path * lot of minor fixes * using dotted relative import syntax everywhere, added tests for registry(redis) * cleaning up queries and imports * cleanup of os.path and more comments on thumbnail * fixed typo * added buildspec.yml file for testing AWS CodeBuild * setting env varialbe manually for aws codebuild * removed buildspec.yml file * showing loading spinner if file is still processing
No description provided.