Skip to content

Conversation

@Kerollmops
Copy link
Member

No description provided.

@Kerollmops Kerollmops added this to the v0.22.0 🔑 milestone Mar 11, 2025
@Kerollmops Kerollmops requested review from dureuill and irevoire March 11, 2025 10:22
@Kerollmops Kerollmops force-pushed the env-default-with-tls branch from 9ca9c26 to c40b23d Compare March 11, 2025 10:26
@dureuill
Copy link
Contributor

Hello, what is the motivation here? I find that having to explicitly indicate which variant of env we want is good and rather unobstrusive

@Kerollmops
Copy link
Member Author

Hey 👋 It removes one breaking change and is similar to the change we did for the RoTxn<T = WithTls>.

Copy link
Contributor

@dureuill dureuill left a comment

Choose a reason for hiding this comment

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

we'll have to be careful when integrating in Meilisearch to define all Env as Env<WithoutTls>. Otherwise, approved

@irevoire
Copy link
Contributor

What happens if we don’t? We should get an error when we try to share a RoTxn through multiple threads, right? It won’t silently crash?

@Kerollmops
Copy link
Member Author

Ahah, I already had to make them Env<WithoutTls> anyway and there are no actual issue if we forgot and require a RoTxn<WithoutTls>, it will simply not compile.

@Kerollmops Kerollmops merged commit 2988ff8 into main Mar 11, 2025
18 checks passed
@Kerollmops Kerollmops deleted the env-default-with-tls branch March 11, 2025 13:10
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