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

Inconsistent representation of cpusets #626

Open
LewisGaul opened this issue Sep 1, 2024 · 3 comments
Open

Inconsistent representation of cpusets #626

LewisGaul opened this issue Sep 1, 2024 · 3 comments
Labels
good first issue Good for newcomers

Comments

@LewisGaul
Copy link
Collaborator

ContainerCLI.create() and ContainerCLI.run() support cpuset_cpus and cpuset_mems as List[int], which is converted to a CLI arg using ",".join(...).

ContainerCLI.update() supports cpuset_cpus and cpuset_mems as List[int] in the type signature but then converts to a CLI arg simply by stringifying (implicitly in utils.run()), which I assume will always fail.

In reality the CLI accepts a cpuset representation, which includes: a range, e.g. "0-3", a selection, e.g. "0,2", a combination of both, e.g. "0-3,7".

I'd suggest it'd make most sense just to support the user specifying this as a string, rather than only allowing the "selection" format via a list. This would mean fixing the type annotation on update() and a backwards incompatible change on create() and run().

@LewisGaul
Copy link
Collaborator Author

@gabrieldemarmiesse how should we handle this backwards incompatible change, assuming you're happy with what I'm proposing? Should the create() and run() methods still accept List[int] and emit a deprecation warning? Or is this something we can just change?

@gabrieldemarmiesse
Copy link
Owner

@LewisGaul do you think it would make sense to have List[int] | str? With List[int] being there when someone is programmatically choosing the cpuset. This option is more powerful than the range and combinaison, and is intuitive.

Let me know if I'm missing something, it's not an option I use often.

@LewisGaul
Copy link
Collaborator Author

@LewisGaul do you think it would make sense to have List[int] | str? With List[int] being there when someone is programmatically choosing the cpuset. This option is more powerful than the range and combinaison, and is intuitive.

Seems reasonable to me

@LewisGaul LewisGaul added the good first issue Good for newcomers label Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants