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

Review _public_ API surface of our Server classes #8190

Open
fjetter opened this issue Sep 15, 2023 · 5 comments
Open

Review _public_ API surface of our Server classes #8190

fjetter opened this issue Sep 15, 2023 · 5 comments
Labels
hygiene Improve code quality and reduce maintenance overhead

Comments

@fjetter
Copy link
Member

fjetter commented Sep 15, 2023

Our server classes (Scheduler, Nanny, Worker) are using many, many non-underscored methods that are treated by developers as if they were private because using them requires rather low level understanding of the internals.

It is entirely unclear for users what is safe to use and it is impossible for developers to assess where a deprecation cycle is warranted.

This can lead to misjudgments like #8189 where the clearly public API (via the Client) was preserved but the semi-public API of the non-underscored scheduler method changed it's signature.

While this is a bit of legwork, we should settle this once and review our API surface and rename/deprecate methods we do not want anybody else to use.

@hendrikmakait hendrikmakait added hygiene Improve code quality and reduce maintenance overhead and removed needs triage labels Oct 5, 2023
@crusaderky
Copy link
Collaborator

I recall a maintainers meeting in the past where we discussed this and I think the consensus was that whatever is on our sphinx documentation is public?

@hendrikmakait
Copy link
Member

I recall a maintainers meeting in the past where we discussed this and I think the consensus was that whatever is on our sphinx documentation is public?

I think that's the current rule of thumb that exists in the heads of a few maintainers (it took me over a year of contributing to Dask for someone to first mention this "rule of thumb" to me). So we shouldn't rely on anybody else knowing about it. I think it might be a good rule to start deprecating things; in the end, we'd only formalize the status quo.

@mrocklin
Copy link
Member

I'm curious to learn more about where/if this is causing user pain. My sense is that this public/private question of the Server class affects very few users (and maybe no one that isn't paid). If so, I would deprioritize this topic and focus energy on other things that do demonstrably affect users.

@mrocklin
Copy link
Member

Although, stepping back, I care much less about this topic in particular (if people really want to make things private here then go for it) mostly I want to communicate the meta idea that we should be spending our time on things that affect many users, especially if that time is paid. I find that being constantly sensitive to the question "what can I do right now that will have the largest positive effect" is both difficult and useful.

@hendrikmakait
Copy link
Member

I'm curious to learn more about where/if this is causing user pain. My sense is that this public/private question of the Server class affects very few users (and maybe no one that isn't paid). If so, I would deprioritize this topic and focus energy on other things that do demonstrably affect users.

I've seen this pop up occasionally among users who were not paid by companies contributing to Dask. In general, I agree with you that we should spend our time on high-impact things. Mostly, I think this effort is a nice activity for those 10 minutes of downtime where there's not much else to do or when you need a mental break and need to do something dull.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hygiene Improve code quality and reduce maintenance overhead
Projects
None yet
Development

No branches or pull requests

4 participants