Skip to content

Conversation

@nightkr
Copy link
Contributor

@nightkr nightkr commented May 6, 2022

Description

Requires stackabletech/secret-operator#125, see that PR for motivation

Review Checklist

  • Code contains useful comments
  • (Integration-)Test cases added (or not applicable)
  • Documentation added (or not applicable)
  • Changelog updated (or not applicable)
  • Cargo.toml only contains references to git tags (not specific commits or branches)

Once the review is done, comment bors r+ (or bors merge) to merge. Further information

@nightkr nightkr marked this pull request as ready for review May 6, 2022 12:25
@nightkr nightkr requested review from a team and soenkeliebau May 6, 2022 12:25
Copy link
Member

@maltesander maltesander left a comment

Choose a reason for hiding this comment

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

LGTM.
Edit: Is Ephemeral(Box<EphemeralVolumeSource>) in the enum really required? I mean we do not store 1000s of elements of that in a list. Whats against #[allow(clippy::enum_large_variants)] (not sure thats exactly how the lint is called)?

@nightkr
Copy link
Contributor Author

nightkr commented May 9, 2022

Yeah the main problem with enum_large_variants is that it inflates the size of that type regardless of which variant we use, which might also make things like moves slower depending on the ABI that the compiler ends up deciding on. If we don't particularly are about that then we can just use use it inline.

@maltesander
Copy link
Member

Yeah the main problem with enum_large_variants is that it inflates the size of that type regardless of which variant we use, which might also make things like moves slower depending on the ABI that the compiler ends up deciding on. If we don't particularly are about that then we can just use use it inline.

Yeah but we do not really move that around or store a lot of elements in a list or sth. I think we can ignore the memory overhead for one item here?

@nightkr
Copy link
Contributor Author

nightkr commented May 13, 2022

bors r+

bors bot pushed a commit that referenced this pull request May 13, 2022

## Description

Requires stackabletech/secret-operator#125, see that PR for motivation



Co-authored-by: Teo Klestrup Röijezon <teo.roijezon@stackable.de>
@bors
Copy link
Contributor

bors bot commented May 13, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Use ephemeral secret-op volumes rather than csi [Merged by Bors] - Use ephemeral secret-op volumes rather than csi May 13, 2022
@bors bors bot closed this May 13, 2022
@bors bors bot deleted the feature/csi-topology branch May 13, 2022 07:42
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