-
Notifications
You must be signed in to change notification settings - Fork 2
issue-29: Add a consumer to process requests in chunks [fix] #32
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
issue-29: Add a consumer to process requests in chunks [fix] #32
Conversation
|
I wonder if the tests are supposed to be passed |
I had a similar issue with the tests not passing initially in #26, but it looks like they ended up passing sometime before Ben merged the branch. If I remember correctly, I think he mentioned that the test suite needs to be refactored in some places. It looks like Isla, the previous scientific programmer co-op, was working on this in #10, but the PR was closed. |
|
|
||
| if nargin < 1 | ||
| chunkSize = 2^29; % default value | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be handy to have a check when the user passes in a value for chunkSize to make sure it's less than the 2^30 bytes limit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I'm not quite sure how to pass in a value for chunkSize. It's not a parameter, so would the user need to run chunkSize = int; on the command line before they run a data product order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Checking the chunkSize value would be helpful. I'll add it.
As for how to set this parameter up, it is done when the class is initialized. At the moment when an instance of this class is created, a user can pass input argument to the class like
chunkSize = 2^29;
consumer = onc.ChunkedResponseConsumer(chunkSize);
response = request.send(uri, options, consumer);I might need to add empty brackets to line 28 onc/+util/do_request.m where the consumer is initialized. Or, to spare people from guessing, I can make passing of the parameter explicit by doing it in the way it is done in the code snippet above.
onc/+onc/ChunkedResponseConsumer.m
Outdated
| end | ||
|
|
||
| methods (Access = protected) | ||
| function bufsize = start(obj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably say function buffsize = start(obj)
onc/+onc/ChunkedResponseConsumer.m
Outdated
| @@ -0,0 +1,96 @@ | |||
| classdef ChunkedResponseConsumer < matlab.net.http.io.GenericConsumer | |||
| %% ChunkedResponseConsumer Consumer with an option to control the size of | |||
| % chucks which a response is processed in. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chunks (minor typo but it's worth fixing)
|
Sorry that it took so long to get back to this PR because of dealing with other tasks |
|
Hmm, now the updated code doesn't pass checks even though the updates are minor |
There was a deployment today that may have interfered with the checks. |
onc/+util/do_request.m
Outdated
| if showInfo, fprintf('\nRequesting URL:\n %s\n', fullUrl); end | ||
| tic | ||
| consumer = onc.ChunkedResponseConsumer; | ||
| chunckSize = 2^29; % half of the max response size that `send` can handle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable should be called and passed in as chunkSize to be consistent
Solves #31
This PR adds the way to handle files grater in size than 1Gb. It adds and utilizes a consumer that processes responses in chunks.
How to test
Download files of various types and sizes.
The code has been already tested with the following request parameters: