feat(#138): move polling to openhim channel config #139
Conversation
mediator/config/index.ts
Outdated
| export const SYNC_INTERVAL = getEnvironmentVariable('SYNC_INTERVAL', '60000'); | ||
| //export const SYNC_INTERVAL = getEnvironmentVariable('SYNC_INTERVAL', '60000'); | ||
| // hard code sync interval to 1 minute because it is hard coded in mediator config | ||
| export const SYNC_INTERVAL = '60000'; |
There was a problem hiding this comment.
with the current implementation, yes they should always be the same because the syn uses the sync interval as a workaround to #141
when that is fixed, the code likely won't reference sync interval anywhere
mediator/src/controllers/openmrs.ts
Outdated
| export async function sync() { | ||
| try { | ||
| const startTime = new Date(); | ||
| startTime.setHours(startTime.getHours() - 1); |
There was a problem hiding this comment.
One hour was supposed to be form how long ago the sync should look for new resources. But it should be configureable, I have added an environment vairable SYNC_PERIOD for this instead.
mediator/src/routes/openmrs.ts
Outdated
There was a problem hiding this comment.
What do you think about adding GET /sync route unit tests? While the code is simple, ensuring the route behaves as expected in both success and failure scenarios is a good idea.
There was a problem hiding this comment.
added src/routes/test/openmrs.sync.test with simple success and failure tests, if/when the sync returns more data, we can add more test conditions.
| export const SYNC_INTERVAL = getEnvironmentVariable('SYNC_INTERVAL', '60000'); | ||
| // hard code sync interval to 1 minute because it is hard coded in mediator config | ||
| export const SYNC_INTERVAL = '60'; | ||
| // how far back shoudl the sync look for new resources |
There was a problem hiding this comment.
Adding the default sync period to the comment in a human-readable manner would be helpful. e.g. How far back should the sync look for new resources. Defaults to 1 hour.
| @@ -21,11 +21,12 @@ const app = express(); | |||
| app.use(bodyParser.json()); | |||
| app.use(bodyParser.urlencoded({extended: true})); | |||
|
|
|||
| /* | |||
There was a problem hiding this comment.
Remove this code rather than commenting it out.
|
|
||
| //Create a patient using openMRS api | ||
|
|
||
| /*const retrieveFhirPatientIdResponse = await request(FHIR.url) |
There was a problem hiding this comment.
Let's replace this commented out code with a TODO.
making PRs against openmrs_mediator instead of just pushing to openmrs_mediator directly, to make review easier
closes #138