tristepin222 commented 6 months ago

From what has been said in BI review and Finalize Backend Services.

you can find the last commit to review here : c02dcca

I had no choice but to convert my backend from .net core to ASP.net core to be able to create my API gateway, that is why, I created a new github repository for the backend which can be found here : Backend.

so here the changes i have made :

Code quality

I did my best to make sure white spaces and useless lines are deleted.
I renamed visionTest, visionTestTest to more appropriate names.
Analyzer was renamed to ILabelDetector.
I personally didn't think the amount of using are problematic, there are only 5 sub namespace below the Vision name space, and most of the files only uses up to 6 namespaces at best, but i do agree it would be a problem if there are more namespaces.

Interfaces

Interfaces should be compliant now.

Tests

variables can be changed under visionTests/Properties/launchSettings.json.

implementation

variables management

it is integrated in launchSettings.json, so it can be changed at will, as long the image exists.
credentials are generated by google, and can be put under backend/env/, or any other folder you wish, but you will need to specify it in backend/Properties/launchSettings.json, under "GOOGLE_APPLICATION_CREDENTIALS" variable.
ImageData isn't used anymore, it returns a json now.

Exceptions

the exception does implement the base exception class, but i noticed google have it's own exception, which makes difficult to try and catch an exception that isn't Exception, that is why at GoogleDataObjectImpl, line 30, i have this: catch (Exception ex) when (ex is ObjectNotFoundException || ex is GoogleApiException).
they're all implementing DataObjectImplException rather than GoogleDataObjectImplException now.

NicolasGlassey commented 5 months ago

@tristepin222

Merci pour ce livrable.

RIA2 Les informations vont m'aider à faire la review, cependant avez-vous bien tout livré dans "main" ?

BI Vous m'indiquez le lien du dépôt en début d'issue, mais quel est le commit à prendre en compte ?

tristepin222 commented 5 months ago

Bonjour,

RIA2 Oui, tout est sur le main

BI

vous pouvez trouver le commit en question 2b886d6

CPNV-ES-RIA2 / JESSY