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

Allow for passing properties to NewSystemd #344

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shell-skrimp
Copy link

I had a need to disable some functionality but couldnt do so with current implementation so I made a tweak that allows for passing custom properties.

Allow for setting properties

Signed-off-by: shell-skrimp <[email protected]>
@dcantah
Copy link
Member

dcantah commented Jul 22, 2024

@shell-skrimp Unfortunately this changes the public API of NewSystemd which would be a breaking change. You could introduce a new "constructor" that takes in the properties. Basically what you have right now just with a new name like NewSystemdWithProperties

@samuelkarp
Copy link
Member

I had a need to disable some functionality

Can you tell us more about your use-case? As @dcantah noted this is a breaking change; a new constructor would be a better option but as of now I don't think we know of a particular use-case for this in containerd.

@shell-skrimp
Copy link
Author

Hey all, sorry I didnt reply sooner, been busy ;)

My use case was in a different project where I needed to create cgroups for a parent and its child processes. I think that @samuelkarp idea to have a new constructor is probably best way to go to prevent breakage. However, I'm not sure if this cgroups project wants to be consumable by other projects or if it only wants to stay in the scope of containerd? If the latter then you can disregard this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants