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

Add new option cpu_weight and user- and group-specific configurations #98

Closed
wants to merge 0 commits into from

Conversation

ticoneva
Copy link

@ticoneva ticoneva commented Aug 19, 2022

This PR introduces two changes:

  1. New configuration option cpu_weight. This sets the systemd property CPUWeight, which causes available CPU time to be sliced in proportion to each process' weight.
  2. Introduces two new configuration options group_config and user_config. These two options accept dict of configuration options specific to individual groups and individual users. For example, for group_config:
    c.SystemdSpawner.group_config = {
       'group_name': {
             'property': value,
             ...
       },
       ...
    }
    Only configurations implemented within systemdspawnwer are supported.

The combination of change 1. and 2. allow group- or user-specific resource allocation. For example, on our academic computing cluster, faculty members have much higher cpu_weight and mem_limit than students.

@welcome
Copy link

welcome bot commented Aug 19, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@consideRatio
Copy link
Member

consideRatio commented May 26, 2023

This PR includes two kinds of features (user/group config and cpu weight), and I think both are relevant.

  • I'd like to see the overwrite_config logic a bit more maintainable, where we don't need to update it each time a new config option is added etc.
  • SystemdSpawner provides various configuration directly, but there is also configuration from the base class that could be considered for updating. What should we do with regards to this?
  • When adding an option to update config for specific users/groups, one may want to either merge or overwrite. In jupyterhub/kubespawner we have opted to provide this feature as well, where we first made it overwrite, but then ended up making it merge config. This meant that dictionaries were updated. There are pro's and con's to each I think, where overwrite is simpler and easier to make robust.
  • I'd like to review these two features in separate PRs to reduce the complexity for me as a maintainer, but its not blocking

If you want to work this still @ticoneva, I'd be happy to invest time reviewing it!

@ticoneva
Copy link
Author

ticoneva commented May 27, 2023

@consideRatio I have been thinking about the issue of maintainable too. This is what I have come up with at this moment:

from traitlets.traitlets import is_trait
...
def overwrite_config(self,source):
    for config_name in source:
        config = getattr(self,config_name)
        assert is_trait(config), f"{config_name} is not configurable."
        setattr(self,config_name,source[config_name])

This should cover all configurable options, since as far as I I tell all spawner configurations are handled through traitlets.

Regarding overwrite vs merge, from what I can see in kubespawner, the current implement appears to be a overwrite rather than a merge?

Splitting the two features into separate PRs is not a problem, I can do that.

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

Successfully merging this pull request may close these issues.

2 participants