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

Increase attachment upload verbosity #17

Closed
deveaud-m opened this issue Dec 20, 2022 · 6 comments · Fixed by #18
Closed

Increase attachment upload verbosity #17

deveaud-m opened this issue Dec 20, 2022 · 6 comments · Fixed by #18

Comments

@deveaud-m
Copy link
Collaborator

Only two scenarios are covered by the API function upload_resource_attachment():

  1. response.ok does not raise
  2. Not response.ok raises SW360Error

If the HTTP response received for this call is 202, the function call simply returns however the attachment has not been uploaded to the e.g. component but a moderation request has been created. Example of the received response:

HTTP status = 202
{
  "message" : "Moderation request is created"
}

In that case the client should be informed about the result of the call instead of expecting that the attachment has properly been transferred.

/cc @t-graf @gernot-h

@gernot-h
Copy link
Collaborator

I think that's a general issue we have in sw360python. I can't exactly recall details, but I've already seen cases where other non-critical issues are also silently ignored.

From the concept of the SW360 moderation requests, I would argue that the attachment has been properly transferred if you get 202 (which is also in the 2xx success range).

As it might take an unpredictable time in practice until the moderation request is acted upon and the user uploading things is confused not to see the attachments, I however agree we should somehow inform the user. But not via an exception as this might abort surrounding code while the operation succeeded in some way.

I think the proper answer here would be to add logging means to our library. Do you have recommendations for this, @deveaud-m?

@gernot-h
Copy link
Collaborator

The other option would be to make details on the actual request available to the caller - something which I also missed already in some cases (e.g. to get the body with the error message in case an error occured). But this would probably break our complete API which is probably not what we want to do. Any ideas?

@deveaud-m
Copy link
Collaborator Author

I would use loguru but this should be done across the whole project and would need to refactor the library to bring more information to the clients.

We can start with the POST /attachments endpoint and see if it brings more value to the callers, and then step by step update the library to expose more information from all relevant endpoints.

I can start with #18 if that makes sense and log a WARNING instead of throwing an error.

@tngraf
Copy link
Collaborator

tngraf commented Dec 20, 2022

202 indicates success (accepted), so I would not expect an expection.
A moderation request telsl the user that he/she does not have the neccessary rights to perfom a certain action, but it also indicates that the request has been forwarded to a person who can decided on this.
The user case see his open moderation requests on the SW360 home screen in the "My Task Submissions" section.
At Siemens I am one of the persons who accepts many of these moderation requests - if they belong to my organization.
If other organizations do not process their moderation requests within an approriate time frame, it might be an process issue and not a tool issue.

Regarding logging: yes, this might be a solution, but if we use normal logging and debug messages, then we get a lot of messages from the request library...

Getting the body of an error message IS possible ...but not straight forward.

@deveaud-m
Copy link
Collaborator Author

Logging messages from the request library can be filtered-out:

import logging

# Only show warnings
logging.getLogger("urllib3").setLevel(logging.WARNING)

# Disable all child loggers of urllib3, e.g. urllib3.connectionpool
logging.getLogger("urllib3").propagate = False

Would this be the preferred way to propagate information from this SW360 library to the clients?

@gernot-h
Copy link
Collaborator

As logging seems to be the way for debugging the lower requests and urllib3 layers, it sounds like a good idea to me to build on logging also for the sw360python abstraction layer. I'd also prefer pure Python logging over adding another dependency here.

So my answer would be yes. 😸

@tngraf, what do you think?

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

Successfully merging a pull request may close this issue.

3 participants