Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Response code 500 for mistyped or missing license in the Compatibility Matrix. #2

Closed
michelescarlato opened this issue Jun 23, 2021 · 10 comments

Comments

@michelescarlato
Copy link
Member

@tmortagne
Dear Thomas, I noticed most of the API Endpoints for Maven projects provide 400 and 404 as error messages, and none of them has 500.

LCV REST APIs provide 500 response codes when a mistyped or missing license in the Compatibility Matrix is POSTed, in both inbound or outbound licenses.

From your perspective, should this be an issue to address?

Small update, I am working on adapting the LCV to read the full OSADL matrix .

@tmortagne
Copy link
Member

tmortagne commented Jun 23, 2021

LCV REST APIs provide 500 response codes when a mistyped or missing license in the Compatibility Matrix is POSTed, in both inbound or outbound licenses.

Unknown resources are generally reported with 404 errors and not 500 which is generally used more for really invalid input (which is not the case here since the license is most probably a valid one, but the server can hardly know all the licenses that exist) or unexpected server side problems.
Now for this specific use case I feel that not taking into unknown licenses for the validation but also report them the response would be much more valuable.

That means that on Maven plugin side I could print a warning about the licenses that could not be taken into account in the result instead of totally making it impossible to validate at least the known ones.

@michelescarlato
Copy link
Member Author

That means that on Maven plugin side I could print a warning about the licenses that could not be taken into account in the result instead of totally making it impossible to validate at least the known ones.

@tmortagne
What about printing a warning showing the licenses that are taken into account?
The plan is to support 66 OSS licenses.
This list could be retrieved directly from the matrix, which wouldn't require you to update the warning after a Matrix update.

@tmortagne
Copy link
Member

tmortagne commented Jun 24, 2021

What about printing a warning showing the licenses that are taken into account?

Not really sure what you mean by "print".

What is needed from the REST API is:

  • do license validation on the licenses known by the service (but if the outbound license itself is unknown you cannot really do much, obviously, so you should return a 404 error in that case for example)
  • some way in the HTTP response to be able to deduce which licenses were taken into account

@michelescarlato
Copy link
Member Author

That means that on the Maven plugin side, I could print a warning about the licenses that could not be taken into account in the result instead of totally making it impossible to validate at least the known ones.

@tmortagne Sorry, I didn't mention pint a warning in the Maven plugin side. I was just following your reasoning.

My point is that we can provide the list of the supported licenses just by looking at the compatibility matrix.
It could be hard to print a list of unsupported licenses (they are unsupported because they are not present in the Matrix, so it would also be difficult to list those).

Printing a warning (as you mentioned in the first reply), including the licenses supported, would make more sense. Furthermore, as I explained, if the Matrix is updated with new licenses, the warning will follow the new Matrix structure.
Does this sound to you?

@tmortagne
Copy link
Member

It could be hard to print a list of unsupported licenses (they are unsupported because they are not present in the Matrix, so it would also be difficult to list those).

I don't think you understand what I suggested. I'm not asking the REST API to indicate the list of all the licenses that it does not support since that's obviously not possible, I'm just asking the REST API to indicate in the response which of the licenses that were sent in the request were taken or not into account.

@michelescarlato
Copy link
Member Author

Now I got it.
I will create a 404 response with the name of the license that is not supported by the matrix.
Sorry for the misunderstanding @tmortagne

I will do my best to try to address your request.

@michelescarlato
Copy link
Member Author

Dear Thomas @tmortagne,

I managed to check upon missing or mistyped inbound and outbound licenses, providing the verbose endpoints with a log also related to these licenses.

Still is open the issue to provide a 404 in some cases. But honestly, at the moment, I don't have in mind what other cases could be generating an error.

I will close this issue whenever we are sure that, in any case, the REST APIs will provide an error.

Thanks for your kind suggestions.
Michele

@tmortagne
Copy link
Member

But honestly, at the moment, I don't have in mind what other cases could be generating an error.

I cannot think of anything else that an unknown outbound license.

Looking forward to testing the new license validation API !

@michelescarlato
Copy link
Member Author

Also, the outbound license is handled.

Well, as I mentioned before, these controls are made upon verbose REST API endpoints, namely, where details regarding each inbound are given.

A few flag endpoints just provide True, False, or DUC. So I didn't cover those endpoints.
Here there are two ideas about those endpoints:

  • providing a response including only the name of the not included license explaining that is not supported, without providing the assessment (probably easier at this point for me).
  • providing a 404 error probably specifying that one (or more) of the licenses are not included in the Matrix.

I would go for the first point, but please feel free to provide your suggestion.

Sincerely,
Michele

PS for testing the new APIs, I am waiting for @mir-am to deploy the lcv-restapi in a way that you can use it.
So far is reachable only from the internal network ( I tried it using monster)

@michelescarlato
Copy link
Member Author

michelescarlato commented Jul 26, 2021

This issue should be fixed by introducing a few functions that verify if a license is in the Matrix or not.
More details upon this approach are provided in:
#4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants