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

Sever client networking cryptographic provider initialisation #670

Merged
merged 13 commits into from
Jun 26, 2024

Conversation

SirCipher
Copy link
Member

No description provided.

Copy link

codecov bot commented Jun 24, 2024

Codecov Report

Attention: Patch coverage is 9.37500% with 29 lines in your changes missing coverage. Please review.

Project coverage is 51.11%. Comparing base (005eda0) to head (ae29730).
Report is 1 commits behind head on main.

Current head ae29730 differs from pull request most recent head 95fcce3

Please upload reports for the commit 95fcce3 to get more accurate results.

Files Patch % Lines
client/swimos_client/src/lib.rs 0.00% 15 Missing ⚠️
server/swimos_server_app/src/server/builder/mod.rs 0.00% 7 Missing ⚠️
runtime/swimos_remote/src/tls/mod.rs 0.00% 6 Missing ⚠️
runtime/swimos_remote/src/tls/config/mod.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #670      +/-   ##
==========================================
- Coverage   51.23%   51.11%   -0.12%     
==========================================
  Files         342      343       +1     
  Lines       29584    29520      -64     
==========================================
- Hits        15156    15088      -68     
- Misses      14428    14432       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@horned-sphere horned-sphere left a comment

Choose a reason for hiding this comment

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

I think it would be better to allow the user to specify the default CryptoProvider rather than adding it to the configuration. As we're providing a library, the Swim server could form part of a larger application where the user is relying on setting the default crypto provider an they want it used everywhere.

We should then either add a builder method to the server, or a feature flag to the crate (probably enabled by default) that will set the CryptoProvider to our choice that can be used by people who don't want to care about it.

@SirCipher SirCipher requested a review from horned-sphere June 26, 2024 10:28
@SirCipher SirCipher requested a review from horned-sphere June 26, 2024 13:55
…t_crypto_provider

# Conflicts:
#	server/swimos_server_app/Cargo.toml
#	swimos/Cargo.toml
@SirCipher SirCipher requested a review from horned-sphere June 26, 2024 14:24
@SirCipher SirCipher merged commit cabecec into main Jun 26, 2024
10 checks passed
@SirCipher SirCipher deleted the client_crypto_provider branch June 26, 2024 14:51
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.

2 participants