migrate pending data sources - #236
Conversation
|
/deploy qa4 |
|
ewollesen updated values.yaml file in qa4 |
|
ewollesen updated flux policies file in qa4 |
|
ewollesen deployed clinic eric-migrate-pending-data-sources branch to qa4 namespace |
32828a8 to
5e352dd
Compare
| // Prepend, so the most recent request is stored first | ||
| "$position": 0, | ||
| update := bson.M{ | ||
| "$push": bson.M{ |
There was a problem hiding this comment.
Did you want to update the updatedTime field? That seems to have been dropped. Or are we considering the provider connection requests a "supplementary" object that doesn't necessarily imply changing the patient itself?
There was a problem hiding this comment.
Also, any concerns with this array being unbounded? Probably not from a real-world-use perspective.
There was a problem hiding this comment.
Also, I know this hasn't changed, but we are storing the ProviderName twice, once as the key and once in the value.
There was a problem hiding this comment.
Or are we considering the provider connection requests a "supplementary" object that doesn't necessarily imply changing the patient itself?
This. Since there's no functional or user visible change to the patient, I'm not touching their updatedTime field.
Also, any concerns with this array being unbounded? Probably not from a real-world-use perspective.
Agreed. I have concerns, but this code predated my changes, and it's a necessary feature to see the history of connection requests.
Also, I know this hasn't changed, but we are storing the ProviderName twice, once as the key and once in the value.
Yeah, it annoys me too sometimes, but other times its proven useful to have it in both places.
There was a problem hiding this comment.
@ewollesen Databricks uses the patient's updated time to pull new changes. updatedTime ought to be updated when a change to the patient object is made, so those changes can be reflected in all dashboards managed in databricks.
There was a problem hiding this comment.
@ewollesen Databricks uses the patient's updated time to pull new changes.
updatedTimeought to be updated when a change to the patient object is made, so those changes can be reflected in all dashboards managed in databricks.
Oh you're right. I was thinking of the migration when I wrote this wasn't a material change, not for when providerConnectionRequests are being added/modified. Will fix.
| // Prepend, so the most recent request is stored first | ||
| "$position": 0, | ||
| update := bson.M{ | ||
| "$push": bson.M{ |
There was a problem hiding this comment.
@ewollesen Databricks uses the patient's updated time to pull new changes. updatedTime ought to be updated when a change to the patient object is made, so those changes can be reflected in all dashboards managed in databricks.
e442b47 to
88950ea
Compare
|
/deploy qa4 |
|
ewollesen updated values.yaml file in qa4 |
|
ewollesen updated flux policies file in qa4 |
|
ewollesen deployed clinic eric-migrate-pending-data-sources branch to qa4 namespace |
8e1f560 to
2cedef5
Compare
toddkazakov
left a comment
toddkazakov
left a comment
There was a problem hiding this comment.
In Xealth we display the state of the data source (see https://github.com/tidepool-org/clinic/blob/master/xealth/program.go#L62-L67). I believe we should change that logic so we can display similar information, but this time based on the provider connection requests.
There was a problem hiding this comment.
This path will still create a pending data source, even though it's not a valid value anymore
There was a problem hiding this comment.
This block should be removed, now that data sources are marked as read only in the spec
3703ff2 to
8fe3b7f
Compare
The expiration time of a pending provider connection belongs to the provider connection request itself, not to a placeholder data source. Expose it as a read-only field on the API, and have the service layer assign both createdTime and expirationTime, as they're business logic decisions that belong in the service layer rather than with callers. BACK-4414
Adding a provider connection request no longer creates or updates a placeholder dataSource with a pending or pendingReconnect state. The provider connection request itself now carries all the information needed to represent a pending connection, so the request is simply prepended, and the patient's updatedTime is bumped. BACK-4414
New xealth orders now create provider connection requests directly, instead of placeholder dataSources with a pending state. BACK-4414
...and mark dataSources as read-only, since they are only ever written by the clinic-worker CDC handler. Existing placeholder dataSources are migrated by the back-4414-migrate-pending-data-sources script in tools-private. BACK-4414
8fe3b7f to
26eef10
Compare
4 participants
BACK-4414