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

Refactor use of common strings to enum #4681

Open
sgaist opened this issue Jan 24, 2024 · 2 comments
Open

Refactor use of common strings to enum #4681

sgaist opened this issue Jan 24, 2024 · 2 comments

Comments

@sgaist
Copy link
Contributor

sgaist commented Jan 24, 2024

Proposed change

There are several strings (user, group, server, service, etc) that are used throughout the code base as keys, for loop items and the likes. These could be considered constants and be moved to one or more enum.

Despite making the code more verbose, it would help avoiding pitfalls usually associated with typos just as wrong keys and would trigger explicit errors.

As an example, the content of the scopes.py where there's a lot of checks and data extraction using such strings.

Alternative options

Don't change anything

Who would use this feature?

Developers of JupyterHub as well as people reading through code for better understanding.

(Optional): Suggest a solution

  • Analyse the strings used in the code base
  • Establish one or more Enum to replace them (for example Resources for user, group, server, service.
  • Update strings use with Enum.
Copy link

welcome bot commented Jan 24, 2024

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@manics
Copy link
Member

manics commented Feb 3, 2024

I think this is worth trying on one of the proposed enum types first, so we can evaluate tradeoffs in how they're implemented. E.g.
__str__ can be overridden to return Enum.item.value instead of the default representation making it easier to use in strings, at the expense of not behaving how Enums normally behave.

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

No branches or pull requests

2 participants