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

Propagate file upload timeout #2439

Merged
merged 8 commits into from
Feb 2, 2023
Merged

Conversation

vaavva
Copy link
Member

@vaavva vaavva commented Feb 1, 2023

Checklist

  • I have read the [contribution guidelines] (https://github.com/Azure/azure-iot-sdk-c/blob/main/.github/CONTRIBUTING.md).
  • I added or modified the existing tests to cover the change (we do not allow our test coverage to go down).
  • If this is a modification that impacts the behavior of a public API
    • I edited the corresponding document in the devdoc folder and added or modified requirements.
  • I submitted this PR against the correct branch:
    • This pull-request is submitted against the main branch.
    • I have merged the latest main branch prior to submission and re-merged as needed after I took any feedback.
    • I have squashed my changes into one with a clear description of the change.

Reference/Link to the issue solved with this PR (if any)

Description of the problem

Setting the option 'OPTION_BLOB_UPLOAD_TIMEOUT_SECS' only persists for step 1 and 3 of the file upload (telling IoTHub there's an upload and telling IoTHub the upload finished)

Description of the solution

This PR makes sure the timeout gets set in step 2 as well (the upload of the file)

@vaavva vaavva changed the title Propogate file upload timeout Propagate file upload timeout Feb 1, 2023
@@ -343,7 +343,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_with_NULL_SasUri_fails)
unsigned int httpResponse = HTTP_OK;

///act
BLOB_RESULT result = Blob_UploadMultipleBlocksFromSasUri(NULL, FileUpload_GetData_Callback, &context, &httpResponse, testValidBufferHandle, NULL, NULL, NULL);
BLOB_RESULT result = Blob_UploadMultipleBlocksFromSasUri(NULL, FileUpload_GetData_Callback, &context, &httpResponse, testValidBufferHandle, NULL, NULL, NULL, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a test where we're passing a non-zero timeout here and make sure that the HTTPAPI_SetOption() is being executed. See the blocks in Blob_UploadMultipleBlocksFromSasUri_various_sizes_happy_path_Impl like say below to give a flavor of how this works with optional certs.

        if (certificate != NULL)
        {
            STRICT_EXPECTED_CALL(HTTPAPIEX_SetOption(IGNORED_PTR_ARG, "TrustedCerts", IGNORED_PTR_ARG));
        }

Copy link
Member Author

Choose a reason for hiding this comment

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

Added!

@CIPop
Copy link
Member

CIPop commented Feb 2, 2023

This is failing because of a CI bug introduced by a new policy.

@CIPop CIPop merged commit f983a80 into main Feb 2, 2023
@CIPop CIPop deleted the vaavva/propogate-file-upload-timeout branch February 2, 2023 18:43
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.

5 participants