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

Add more granular upload-to-blob functions, refactor u2b code #2498

Merged
merged 30 commits into from
Jul 20, 2023

Conversation

ewertons
Copy link
Contributor

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

Customers cannot retry notifying Azure IoT Hub of upload completion if any failure on that step occurs.
Also adding a retry mechanism hidden in our code would not allow customers to tailor the retry to their specific needs.

Description of the solution

Expose functions in Azure IoT C SDK for customers directly create, upload and notify completion as they need.

Copy link
Member

@CIPop CIPop left a comment

Choose a reason for hiding this comment

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

I've left a few comments.

One interesting architectural change would be to add a way to extract the URI/SAS token (constructed now internally) that could be then passed to an external Azure Blob SDK (C++ or other tool such as the high-perf az_copy CLI tool).

@@ -355,7 +355,10 @@ extern "C"
* This function is expected to be used along with:
* `IoTHubDeviceClient_CreateUploadContext`
* `IoTHubDeviceClient_AzureStoragePutBlock`
* `IoTHubDeviceClient_AzureStoragePutBlockList`
Copy link
Member

Choose a reason for hiding this comment

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

I would rather move the list of how-to things either in the DevDoc or in a header (a single block, instead of copy/pasting everything) then referencing that doxygen block.

Copy link
Member

@CIPop CIPop left a comment

Choose a reason for hiding this comment

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

Left a few observations about enhancing documentation.

The E2E code has some odd for loop that runs once: I assume that needs to be removed although the comment states otherwise.


attemptCount = 1;

while (true)
Copy link
Member

Choose a reason for hiding this comment

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

❤️

@ewertons ewertons merged commit 979d071 into main Jul 20, 2023
@ewertons ewertons deleted the ewertons/u2b branch July 20, 2023 21:12
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.

4 participants