-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
[Merged by Bors] - Initial work on config merging #368
Conversation
Signed-off-by: Sönke Liebau <[email protected]>
We might want to reorganize this into a "virtual root crate + members" layout, but I'd rather split that into a separate PR to avoid cluttering the diff view too much. |
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.
Im still on the code, just 2 findings to not get lost.
stackable-operator-derive
needs to be added to exceptions in the deny.toml.
Hopefully cargo deny should be happy now... |
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.
LGTM. Works fine for the usecases i could come up with.
As discussed with @teozkr, im fine with merging for now and see what we stumble across later on.
I've been experimenting a bit further with this in feature/merge-experiments, but never really got to a point that I was happy with there. In particular:
I'm starting to believe that " |
## Description - Renamed ConnectionDef to S3ConnectionDef for consistency - Added a `resolve` function to the S3ConnectionDef, similar to the function on the S3BucketDef - minor documentation fix
- added TlsAuthenticationProvider as discussed in stackabletech/documentation#186 - removed MutualTlsVerification Co-authored-by: Malte Sander <[email protected]>
## Description *Please add a description here. This will become the commit message of the merge request later.*
Signed-off-by: Sönke Liebau <[email protected]>
I have tested the code in here with my draft PR in NiFi and everything works as expected. I'd be happy to merge this. However, I f****d up when trying to merge main into this PR earlier today and may have broken other code, not sure.. |
bors r+ |
## Description This is a starting point for #362, but is intended to be usable outside of that context (in this case, the primary motivation for getting this done now is #365). Co-authored-by: Teo Klestrup Röijezon <[email protected]> Co-authored-by: Felix Hennig <[email protected]> Co-authored-by: Malte Sander <[email protected]> Co-authored-by: Sebastian Bernauer <[email protected]> Co-authored-by: Sönke Liebau <[email protected]>
Pull request successfully merged into main. Build succeeded: |
…able (#265) ## Description Fixes #252 This doesn't work with the currently released version of the operator-rs, and is only intended for code study, as this might serve as a template for making similar changes in other operators. Most notably it depends on stackabletech/operator-rs#368 and a PR to make small changes to the Resource structs to make them work with the Merging code, which can in turn only be created once the above mentioned PR has been merged. *Please add a description here. This will become the commit message of the merge request later.* Co-authored-by: Razvan-Daniel Mihai <[email protected]>
Description
This is a starting point for #362, but is intended to be usable outside of that context (in this case, the primary motivation for getting this done now is #365).
Work Checklist until R4R
u8
orString
Review Checklist
Once the review is done, comment
bors r+
(orbors merge
) to merge. Further information