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

feat(attachments): log warning if POST attachment returns 202 #18

Merged
merged 1 commit into from
Dec 23, 2022

Conversation

deveaud-m
Copy link
Collaborator

Not sure if throwing an error is the right thing to do here but not informing the client, that the upload couldn't be performed is not a good choice. Since there is no logging configured in this module, I went for raising ;)

The API documentation released on SW360 servers is unfortunately not very verbose when it comes to POST responses for the /attachments endpoints, so I wrote a test based on experimental data.

Closes #17

@gernot-h
Copy link
Collaborator

gernot-h commented Dec 20, 2022

@deveaud-m, thank you for the PR! However, I don't really agree here. HTTP 202 means that "The request has been accepted for processing, but the processing has not been completed.". I assume the SW360 developers deliberately chose a return code in the "2xx success" range, so we should also consider it a success.

Judging from your test case, I guess this is because you're uploading to an existing release but you have no clearing admin rights in SW360. So it doesn't mean that the upload wasn't performed, it's just waiting for confirmation from the release owner. So I think an exception is not the way to react on this.

There's nothing you as user can do to fix it.

@tngraf, what do you think?

@deveaud-m
Copy link
Collaborator Author

Judging from your test case, I guess this is because you're uploading to an existing release but you have no clearing admin rights in SW360. So it doesn't mean that the upload wasn't performed, it's just waiting for confirmation from the release owner. So I think an exception is not the way to react on this.

I was expecting this feedback.

We need at least to inform the client that a moderation request has been created but there is no logging in this python module and using a print() is not ideal.

@gernot-h What would you propose?

@gernot-h
Copy link
Collaborator

Sorry, I've seen #17 too late, we should probably continue discussion there.

@deveaud-m
Copy link
Collaborator Author

@gernot-h @tngraf patch has been updated according to your feedback from #17

@deveaud-m deveaud-m changed the title feat(attachments): throw SW360Error if POST attachment returns 202 feat(attachments): log warning if POST attachment returns 202 Dec 21, 2022
@tngraf tngraf merged commit b73f6cd into master Dec 23, 2022
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 this pull request may close these issues.

Increase attachment upload verbosity
3 participants