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

The mutability of the API 0.6.4, is it by design? #155

Closed
Forsworns opened this issue Dec 19, 2023 · 4 comments · Fixed by #164
Closed

The mutability of the API 0.6.4, is it by design? #155

Forsworns opened this issue Dec 19, 2023 · 4 comments · Fixed by #164

Comments

@Forsworns
Copy link

Forsworns commented Dec 19, 2023

I found that the LinuxMemory is without setter, is this by design?

BTW, the current API returns an immutable reference to the struct fields, making it hard to revise the spec in place. Why not simply return a mutable reference, so that we can use get_or_insert to modify it.

For example, I have a mutable referenced spec s and want to only modify its cpu quota in palce. With current API (v0.6.4), I have to write the following code, right?

let mut linux = s.linux().clone().unwrap_or_default();
let mut resources = linux.resource().clone().unwrap_or_default();
let mut cpu = resources.cpu().clone().unwrap_or_default();
cpu.set_quota(Some(new_cpus));
resources.set_cpu(Some(cpu));
linux.set_resources(Some(resources));
s.set_linux(Some(linux));

So complicated :(

@Forsworns
Copy link
Author

Friendly ping @utam0k

@utam0k
Copy link
Member

utam0k commented Dec 24, 2023

@Forsworns
Copy link
Author

@utam0k Thanks, but I only want to modify a single field of existing spec, as described above, the spec.linux.resource.cpu.quota.
It is nested so deep that it seems not a good choice to use the builder pattern, I have to copy each attribute of thespec to a new builder. Do I misunderstand something?

SpecBuilder::default()
        .root(s.root().unwrap_or_default())
        .process(
            s.process().unwrap_or_default()
        )
        .linux(
               LinuxBuilder::default()
                      .resources(
                                 LinuxResourceBuilder::default()
                                         .cpu(...)
                                 ...
                      )
                      ...
               ).build()?
        )
        ...
        .build()
        .context("failed to create spec")

@Forsworns
Copy link
Author

Forsworns commented Dec 25, 2023

I guess simply appending a get_mut="pub" to the current getset attributes is enough.
I can take this if you think it's worthwhile to do so. @utam0k

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 a pull request may close this issue.

2 participants