-
-
Notifications
You must be signed in to change notification settings - Fork 2
AWS Signature Version 4 #149
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
base: main
Are you sure you want to change the base?
Conversation
7c76652
to
9cc3e09
Compare
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.
Few comments
nanoFramework.Aws.IoTCore.Devices/AwsSignatureVersion4/nfArrayListExtensions.cs
Show resolved
Hide resolved
/// </summary> | ||
/// <param name="items">Items in the ArrayList</param> | ||
/// <param name="comparer">The Comparer</param> | ||
public static void Sort(this ArrayList items, IComparer comparer) |
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.
now I get your question on Discord :-)
Why not using a quick-sort algorithm? It's fast and simple: https://www.geeksforgeeks.org/quick-sort/
You even have it implemented directly in C# there.
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.
Thanks, I will look into that as a later optimization. Given it is currently working as expected (in the context of my pesudo tests matching full fat .Net), I would rather not mess with it until appropriate unit tests are added and fully end to end tested.
SonarCloud Quality Gate failed.
|
Before we get real dirty.
Re-comment out un-necessary classes
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
@coderabbitai full-review |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@networkfusion I'll conduct a full review of this PR. Let me first gather information about the changes made. 🏁 Script executed:
Length of output: 93270 🏁 Script executed:
Length of output: 810 🏁 Script executed:
Length of output: 1711 🏁 Script executed:
Length of output: 849 🏁 Script executed:
Length of output: 1248 🏁 Script executed:
Length of output: 767 🏁 Script executed:
Length of output: 7554 🏁 Script executed:
Length of output: 21282 @networkfusion Thank you for this substantial contribution! I've completed a full review of PR #149, which adds AWS Signature Version 4 support to enable MQTT over WebSockets and broader AWS service integration. 🎯 Overall AssessmentThis is a significant and valuable feature addition that implements AWS Signature Version 4 authentication. The functionality is well-structured with a proper inheritance hierarchy using
|
Description
Adds AWS Signature Version 4, this can be used instead of TLS certificates and will be required for supporting MQTT over WebSockets and HTTP Client support.
If can also be used for other AWS capabilities (e.g. S3 buckets).
It is not fully tested and could probably be optimized, but this is a major attempt towards support.
Motivation and Context
Builds upon #148
How Has This Been Tested?
Screenshots
Types of changes
Checklist: