Gravitate-Health / focusing-manager

Apache License 2.0
1 stars 0 forks source link

lense list should be optional #4

Closed gmej closed 1 year ago

gmej commented 1 year ago

If no lens list is provided, all lenses must be executed.

10alejospain commented 1 year ago

Good job! @aalonsolopez

jkiddo commented 1 year ago

@aalonsolopez looks good so far - doing a POST to https://fosps.gravitatehealth.eu/focusing/focus/bundlepackageleaflet-2d49ae46735143c1323423b7aea24165 with IPS contents now ends up with {"message":"Error in lens execution","reason":{}} - its a shame it does not output the reason.

aalonsolopez commented 1 year ago

Wow, maybe an error uploading the image to the cluster. I can give a quick look at home. Sorry for the inconveniences 😓

aalonsolopez commented 1 year ago

Wait, @jkiddo Is it a matter of the error message being incomplete or the request not working for you? Because I've tested it and it works properly for me

jkiddo commented 1 year ago

@aalonsolopez That is hard to say when there isn't any error message besides the part I posted above.

aalonsolopez commented 1 year ago
catch (error) {
            console.log(error);
            res.status(HttpStatusCode.InternalServerError).send({
                message: "Error in lens execution",
                reason: error
            })
            return
        }

In fact that error is being caught already but it doesn't seem to be working. It's an error displayed when the lenses are failing, due to a failure with either the ePI or the IPS because it might be incorrect or even both of them could not exist in the FHIR Server (if you make an ID search).

That is why I asked you if it was a forced error since despite taking into account that the error messages were not the best, we prefer to focus on finishing the main functionalities of the Focusing Manager and then improve everything to make it much more professional. If you want to open an issue in which you reflect the problem of error messages to keep an eye on everyone, go ahead and I'll get to it as soon as possible. If instead, it is an error because there is something related to the optionality of the lenses that do not work, I reopen this issue and start to investigate as soon as possible.

Thank you very much for everything @jkiddo

PD: We are currently working on a solution to use the Identifier search.

jkiddo commented 1 year ago

@aalonsolopez when will the identifier search be ready?

jkiddo commented 1 year ago

To make it clear: I'm stuck as long as I cant use that Identifier. If need be, then I can temporarily look it up myself on https://fosps.gravitatehealth.eu/epi/api/fhir - and find the id, but even using the id, the invocation still fails.

aalonsolopez commented 1 year ago

I can make the identifier Search for the direction you sent, but with the id should work fine. We've been using the focusing to test the lenses, and it was working as expected

aalonsolopez commented 1 year ago

@aalonsolopez when will the identifier search be ready?

I'm pretty sure that it could me done next week, because this week has been entirely dedicated to lense development. We've been also looking the error messages, making the lenses able to throw errors, but still on it.

jkiddo commented 1 year ago

@aalonsolopez - Okay ... I think I found the issue and I'm fairly confident on it as well. All my code is generated from your spec - and the issue is that the spec lacks expression. On https://github.com/Gravitate-Health/focusing-manager/blob/754fbeb1629a7b14b7539a80a86aa358d76efd71/openapi.yaml#L141 you specify that the IPS input is just a string - not a json object. Due to this, the code I get generated will result in an object that is just something that contains a string. As I put an already json formatted string into it, the deserializer does its job and escapes all "meaning that what I eventually send is escaped json which is not what you expect - (that is at least the impression I get when trying out the samples from the POSTman collection).

TL;DR - you need to fix your openAPI spec and also consume and generate code from it yourself or else you won't understand what issues 3.party such as me ends up with.

jkiddo commented 1 year ago

Here's the fix: https://github.com/Gravitate-Health/focusing-manager/pull/11

jkiddo commented 1 year ago

At least for now makes it work by not trying to respecific the entire FHIR model

aalonsolopez commented 1 year ago

We can create an issue to dedicate some time to fix the OpenAPI spec file, what do you think? And what about this issue? Should be closed too?

jkiddo commented 1 year ago

It can be closed. It works for me now

jkiddo commented 1 year ago

@aalonsolopez FYI,FWIW - the service works for me now and I look up on the FHIR server for the id based on the identifer so I do that processing on my side for now which might be just fine.

jkiddo commented 1 year ago

for the document that is

aalonsolopez commented 1 year ago

perfect, if you need something else please tell us anytime you need, and thank you for everything!