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

Optimize Image upload to avoid buffering. Add cluster related fields in image upload resource & data source for PC #432

Merged

Conversation

bhatipradeep
Copy link
Contributor

@bhatipradeep bhatipradeep commented Apr 27, 2022

New additions:

  1. Directly used os.File as input to http.NewRequest() and added implementation for setting content length & GetBody values for req{} as there is no implementation in http.NewRequest() for os.File type .
  2. Added cluster related fields in image upload resource and data source for PC to upload image to particular cluster in multicluster setup

@bhati-pradeep bhati-pradeep changed the title Optimize Image upload to avoid using buffer Optimize Image upload to avoid buffering Apr 27, 2022

// Set req.ContentLength and req.GetBody as internally there is no implementation of such for os.File type reader
// http.NewRequest() only sets this values for types - bytes.Buffer, bytes.Reader and strings.Reader
// Refer https://github.com/golang/go/blob/a0f77e56b7a7ecb92dca3e2afdd56ee773c2cb07/src/net/http/request.go#L896
Copy link

@WWhitt WWhitt Apr 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: While this is true for those data types, it doesn't seem necessary when using an *os.File, see my example in the issue -- it uploaded perfectly fine without pre-emptively specifying content-length. That being said, I totally support not relying on default behavior here.

Edit: while I did get uploads without doing that, it wasn't working how I thought. ignore me. I agreed with explicitly setting it anyway

@bhatipradeep bhatipradeep changed the title Optimize Image upload to avoid buffering Optimize Image upload to avoid buffering. Add cluster related fields in image upload resource & data source for PC Apr 28, 2022
@bhati-pradeep bhati-pradeep linked an issue Apr 28, 2022 that may be closed by this pull request
@siddharth-nutanix
Copy link
Collaborator

siddharth-nutanix commented Apr 29, 2022

/ok-to-test

Acceptance test run status: failure

@bhati-pradeep
Copy link
Collaborator

Testcase failed due to setup config mismatch. Passing locally after fixing. Merging it to master.

Attached the screenshot.

Screenshot 2022-04-29 at 1 18 25 PM

@bhati-pradeep bhati-pradeep merged commit e9691c6 into nutanix:master Apr 29, 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
5 participants