search

RADAR-base / RADAR-PushEndpoint

A gateway to expose endpoints for Push based services to POST data to RADAR-Base platform.
Apache License 2.0
2 stars 2 forks source link

Error when end date for user is null #64

Open mpgxvii opened 8 months ago

mpgxvii commented 8 months ago

@yatharthranjan Can we allow end date to be nullable or set it to a default value? What do you think?

afolarin commented 8 months ago

I seem to recall we had this discussion before, and I'm not sure if there's a straightforward workaround. For now, perhaps the way to handle this is to use a very far in the advanced future end date?

On Mon, 4 Mar 2024 at 14:21, Pauline Conde @.***> wrote:

  • In the latest versions of rest-sources-auth, we now allow null end date (which may be applicable for some studies where data collection is ongoing and there is no specific end date)
  • However this causes an error in pulling Garmin data through the PushEndpoint, and this affects the processing and pulling of the data. Some error logs:

Failed to parse JSON: com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException: Instantiation of [simple type, class org.radarbase.push.integration.garmin.user.GarminUser] value failed for JSON property endDate due to missing (therefore NULL) value for creator parameter endDate which is a non-nullable type

@yatharthranjan https://github.com/yatharthranjan Can we allow end date to be nullable or set it to a default value? What do you think?

— Reply to this email directly, view it on GitHub https://github.com/RADAR-base/RADAR-PushEndpoint/issues/64, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS72VNMWDW4CPL7CHWSTPDYWR7QLAVCNFSM6AAAAABEFJLV6SVHI2DSMVQWIX3LMV43ASLTON2WKOZSGE3DMOJWHEYTGOI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

--

mpgxvii commented 8 months ago

I seem to recall we had this discussion before, and I'm not sure if there's a straightforward workaround. For now, perhaps the way to handle this is to use a very far in the advanced future end date?

@afolarin Yes, because of this error, it affected the whole application and wasn't able to process incoming data. So I just set the end date for that user and it got resolved. Yes either maybe a default max end date if it is null?

afolarin commented 8 months ago

Yes, that makes sense. Could set default for null it a few centuries later 😅

On Mon, 4 Mar 2024 at 14:36, Pauline Conde @.***> wrote:

I seem to recall we had this discussion before, and I'm not sure if there's a straightforward workaround. For now, perhaps the way to handle this is to use a very far in the advanced future end date?

@afolarin https://github.com/afolarin Yes, because of this error, it affected the whole application and wasn't able to process incoming data. So I just set the end date for that user and it got resolved. Yes either maybe a default max end date if it is null?

— Reply to this email directly, view it on GitHub https://github.com/RADAR-base/RADAR-PushEndpoint/issues/64#issuecomment-1976724479, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS72VNKCHY3OFWSGIMGHVLYWSBHBAVCNFSM6AAAAABEFJLV6SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZWG4ZDINBXHE . You are receiving this because you were mentioned.Message ID: @.***>

--

yatharthranjan commented 8 months ago

I think there are a few things to consider -

  1. If this is a use-case, where we want unbounded end dates, then we should add support for it in the push endpoint
  2. if we think this is not a valid use-case for this app, we can allow null dates in the data class but filter out the users in the user repository when they are being queried.

IMO number 1 but it might take a while to add this, we can add the fix mentioned number 2 in the short term.

afolarin commented 8 months ago

For 1. What do we think the consequences of unbounded end dates are here (other than perhaps some info governance, though the main controls for dictating if data is collected should ideally be the Management Portal as this overides all IIRC). For 2. I guess what is the overhead here and how much operational complexity is added by the filtering.

yatharthranjan commented 8 months ago

For 1. What do we think the consequences of unbounded end dates are here (other than perhaps some info governance, though the main controls for dictating if data is collected should ideally be the Management Portal as this overides all IIRC).

Yes would be ideal if it was all centralised in the MP, but currently there are not controls in MP to specify the end date of a subject For the consequences, i am not sure, apart from added code complexity (one more case to handle in all downstream apps that use rest-sources-auth). This functionality is already present for researchers tbh (like you said someone can just specify very high end date in the authorizer and it would be same). I think this just adds another human error component which is not really needed. Maybe something to discuss with the hyve too.

For 2. I guess what is the overhead here and how much operational complexity is added by the filtering.

Not much complexity i guess apart from us having to followup with people who have left unbounded dates when we believe the study should have ended (but it is similar to someone specifying dates that are too far in the future). This should be somehow limited if people are following instructions provided by us.