-
Notifications
You must be signed in to change notification settings - Fork 19
Conversation
please take a look |
latitude = mongoengine.DecimalField() | ||
longitude = mongoengine.DecimalField() | ||
model_name = mongoengine.StringField() | ||
meta = { |
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.
Empty line before this.
please take a look |
) | ||
else: | ||
models.Precipitation.objects( | ||
created=datetime.strptime(weather_object.attrib['created'], "%Y-%m-%dT%H:%M:%SZ"), |
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.
Constant string. You should use '
.
Please update from main repository. |
We had to change way periodic tasks are run because they were piling up. Sorry for additional work. So now we do not define periodic tasks in settings.py anymore. |
|
||
latitude = self.request.node.latitude | ||
longitude = self.request.node.longitude | ||
#TODO: Check and possibly optimize |
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 put space before TODO
keyword.
Otherwise it is great now! Super! |
bump |
# Fetching data only once per possible duplicate locations | ||
for latitude, longitude in {(node.latitude, node.longitude) for node in nodes.get_all_nodes()}: | ||
weather_tasks.append(update_weather.s(latitude, longitude)) | ||
return celery.group(weather_tasks)()@task.task(rate_limit=20) # 20 tasks per second. Limitation by the api http://api.yr.no/conditions_service.html |
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 invalid syntax.
You can see that by tests which have also failed because of this (GitHub is showing them just bellow the pull request description).
Otherwise it looks good. But we should be doing tests too. Check how I made tests for horoscope. You can make them, or open a ticket for them. You have done a lot already. |
updated, still some bug in test(frozen) but i think it's not a problem in code |
Merged. Tests to be done with #326. |
It seems tests are still failing, there is still bug in the code. Grr. You should really try your code before pushing! https://travis-ci.org/#!/wlanslovenija/PiplMesh/builds/2686585 |
maybe you haven't seen my comment e6c6182 |
#210