V0.3.7 api refactor #528
V0.3.7 api refactor #528
Conversation
fredkingham
force-pushed
the
v0.3.7-api-refactor
branch
from
266e02a
to
5c50a77
Compare
intrahospital_api/loader.py
Outdated
| from intrahospital_api import logger | ||
|
|
||
| api = get_api() | ||
| from intrahospital_api import get_api |
There was a problem hiding this comment.
Can we have all the from intrahospital_api import x statements on one line ?
| batch.start() | ||
| def initial_load(remaining=False): | ||
| """ | ||
| Runs an initial load. |
There was a problem hiding this comment.
This isn't a great docstring comment - it assumes I know what running an initial load is, when I would do it etc...
| demographics_row = DemographicsRow(db_row) | ||
|
|
||
| demographics_row["hospital_number"] == "1" | ||
| demographics_row["name"] == "Wilma Flintstone" |
There was a problem hiding this comment.
I definitely think it's weird to access @propertys this way...
elcid/settings.py
Outdated
| @@ -196,7 +200,6 @@ | |||
| database=None, | |||
There was a problem hiding this comment.
FWIW, Django tends to do these as shouty ALL_CAPS keys....
|
|
||
| if time_ago.seconds < MAX_ALLOWABLE_BATCH_RUN_TIME: | ||
| logger.info( | ||
| "batch still running after {} seconds, skipping".format( |
There was a problem hiding this comment.
What do we "skip" at the point of this log message?
| def get_batch_start_time(service_name): | ||
| """ | ||
| If no previous successful batch run has started. | ||
| Use the started timestamp of the first Initial Patient Load |
There was a problem hiding this comment.
First ? I thought there was never > 1 ?
|
|
||
| if current_running.count() > 1: | ||
| # we should never have multiple batches running at the same time | ||
| raise BatchLoadError( |
There was a problem hiding this comment.
You prefer to do your own checks and raise your own errors not have e.g. a Meta unique_together constraint ?
There was a problem hiding this comment.
So its not unique together on state as we should have lots of batch runs that are successful. However you're right, we don't want to blow up here, we want to blow up when duplicates are added. I'll put an error in the save method
| If no previous successful batch run has started. | ||
| Use the started timestamp of the first Initial Patient Load | ||
|
|
||
| If a previous batch load has run and there was an initial patient |
There was a problem hiding this comment.
Can we have a diagram that explains this ?
I'm only 75% sure I follow the text :)
…every new cron job
…al observations for the time being
…id links into the email
… adds in a unit test for the refresh patient function
…l need to be resolved
…moke check has started
…ab test we are in at this point as otherwise we get the meta class
…un for quite some time
So this is the first (and the worst) of the api refactor pull requests.
Old state was
New State
baseservice provides utilities to help the services./backend/directory and a live.API and a dev.API that is used depending on a setting.service.pythat requests data from the api and saves.