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

[wip] Fix: Delete requests need headers to be included in Batch #66

Closed
wants to merge 1 commit into from

Conversation

ChengQian1007
Copy link

Delete requests need headers to be included in Batch

Ref to Issue: #65

Delete requests need headers to be included in Batch
@CLAassistant
Copy link

CLAassistant commented Oct 10, 2019

CLA assistant check
All committers have signed the CLA.

@filak-sap
Copy link
Contributor

@ChengQian1007 Thank you for taking the time to analyze the problem and post the patch. Do you think you can add/update a test case for this change?

@filak-sap filak-sap closed this Oct 11, 2019
@filak-sap filak-sap reopened this Oct 11, 2019
@filak-sap
Copy link
Contributor

@ChengQian1007 I am sorry but I cannot find any reference for your statement. Which OData service implementation do you use?

@filak-sap
Copy link
Contributor

D'oh, I've just noticed your issue #65. Well, I am not sure if every Batch request must send some Headers and if it's a good idea to add a random dummy headers just to work around a weak implementation of Batch request generator.

What about to either return and empty dict or better detect None in the function encode_multipart?

@ChengQian1007
Copy link
Author

@filak-sap I use CDS View generated OData Service.
Thanks for the advices, I will find out whether a normal Delete Request contains header.

@filak-sap filak-sap added bug Something isn't working wip and removed bug Something isn't working labels Oct 23, 2019
@filak-sap filak-sap changed the title Fix: Delete requests need headers to be included in Batch [wip] Fix: Delete requests need headers to be included in Batch Oct 23, 2019
@phanak-sap
Copy link
Contributor

Closing because this is forgotten PR.. and most probably wrong (does not take into account possibility of Batch requests for data in ATOM format)

@phanak-sap phanak-sap closed this Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants