community icon indicating copy to clipboard operation
community copied to clipboard

gpuCI admin team for Dask and Dask-contrib orgs

Open charlesbluca opened this issue 3 years ago • 13 comments

As pointed out on dask/distributed#5154, it is not immediately obvious to Dask contributors who the gpuCI admins are; currently, the admin list is:

charlesbluca
quasiben
pentschev
jacobtomlinson
jrbourbeau
jsignell
martindurant
jcrist
jakirkham
mrocklin
TomAugspurger

Which doesn't match up with the dask/gpu team that is typically pinged on PRs that need gpuCI approval. @jakirkham suggested that we might improve transparency here by creating a org team that coincides with the admin list; I generally think this a good idea, though I am not sure if everyone on the current admin list would want to get pinged whenever PR approval is needed. I think a sensible solution for now would be to add all the missing members of the dask/gpu team to the current admin list, and then we can have a follow-up discussion on if there's anyone we should remove from the admin list in advance before making a new team.

@jakirkham also suggested doing the same for dask-contrib in preparation for setting up gpuCI on dask-sql. Not sure how folks feel about that, since dask-contrib only has one member currently.

charlesbluca avatar Sep 20 '21 15:09 charlesbluca

Thanks @charlesbluca!

we might improve transparency here by creating a org team that coincides with the admin list

IIRC GitHub org team members are only visible to GitHub users who are a member of the org already. So it depends who we're wanting to provide transparency about the gpuCI admins to. If it's to "all Dask GitHub org" members, then a org team would be a nice way to accomplish this. If it's to "Dask contributors", then a org team probably isn't the right approach

jrbourbeau avatar Sep 20 '21 22:09 jrbourbeau

I get the sense that this information provided to an arbitrary contributor might not be that useful (though could be wrong about that). My guess is someone who is more involved with the project will intercede (so a Dask member), but they might not know who to ask. This team would give them a single contact point. It would also avoid single bus factor issues. Additionally we could put the team name in the docs so people know where to turn.

jakirkham avatar Sep 20 '21 22:09 jakirkham

Yeah I agree with @jakirkham's point - I'm thinking more transparency in terms of general Dask org members rather than all contributors. ATM, I've mostly seen Dask org owners as the primary members interceding to ask the gpuCI admins for approval - I think that putting the team name in the docs (and/or pinning an issue in a relevant Dask repo explaining this team's purpose) could go a long way in increasing awareness of its existence across the general org.

From there, making sure the team is somewhat consistent with the gpuCI adminlist (i.e. no one is in the team that isn't on the adminlist) would just ensure that people aren't being pinged for approval unnecessarily.

charlesbluca avatar Sep 30 '21 14:09 charlesbluca

Additionally we could put the team name in the docs so people know where to turn.

This wouldn't help, because only Dask organization members can view or @ mention those teams. James is right, "visible" only means visible to people who are already organization members https://docs.github.com/en/organizations/organizing-members-into-teams/changing-team-visibility

(When I first started working for Dask this year, that was a problem for me. I got told to look at the teams to figure out who to ask questions... but I didn't have access to any of that information. So please don't put that suggestion in the docs, it doesn't work out well if you don't already have org membership)

GenevieveBuckley avatar Oct 01 '21 02:10 GenevieveBuckley

Yes GitHub has things set a certain way and we are more-or-less stuck with that. Not much we can do there

It likely depends on how we document it. We can have a maintainers section in the documentation targeted towards maintainers. Other projects have similar things as well. Should add that is one of the items tracked in the documentation refresh issue ( https://github.com/dask/community/issues/170 )

jakirkham avatar Oct 01 '21 03:10 jakirkham

I think the suggestion of adding the maintenance team to the docs makes a lots of sense. Probably the steering council should also be on there.

jsignell avatar Oct 01 '21 15:10 jsignell

So, is there a link to an up-to-date list of the people with GPU CI permissions? One that will automatically still be up to date next year, and the year after that, etc.?

There's not much point adding a brittle text list that won't get updated correctly. Incorrect info in the docs would be worse than no information, imo

GenevieveBuckley avatar Jul 22 '22 06:07 GenevieveBuckley

(Not wanting to pull this thread in another direction -- happy to open a separate issue if preferred)

There are several folks who have commit rights to dask/dask and dask/distributed that aren't on the gpuCI admin list. I'd like those folks to be able to shepherd PRs through independently and make sure that GPU-related tests pass.

dask/dask:

  • @crusaderky
  • @douglasdavis
  • @gjoseph92
  • @ian-r-rose
  • @ncclementi
  • @pavithraes
  • @fjetter
  • @rjzamora (if he's not already on the list)

dask/distributed:

  • @crusaderky
  • @gjoseph92
  • @fjetter
  • @ian-r-rose
  • @ncclementi

@charlesbluca @quasiben any objection to adding these maintainers to the list?

jrbourbeau avatar Aug 05 '22 22:08 jrbourbeau

@jrbourbeau I recently went through the effort of combing through these lists, comparing them to those with perms on gpuCI (in a private repo unfortunately), and updated these. Just looked through the list that you have compiled here (thanks for doing that! 🙏) and verified all of them already have permissions based on that last update. Now Idk if we are including this list of users in other places (docs, for example), but would go ahead and add those people to any other relevant lists

jakirkham avatar Aug 05 '22 22:08 jakirkham

Should add RAPIDS is looking into a different CI system in the future that will be able to pick up users and add perms to CI based on GH perms. So this should alleviate some duplication of info/maintenance in the future

jakirkham avatar Aug 05 '22 22:08 jakirkham

@jakirkham 🙌 Thanks for doing this I wasn't aware that this was updated. I've just tested it and it worked.

ncclementi avatar Aug 05 '22 22:08 ncclementi

Yeah sorry I meant to update this thread about that change, but got a bit distracted.

Glad to hear it is working 😄 If anyone runs into issues, please let us know and we can take another look.

jakirkham avatar Aug 05 '22 22:08 jakirkham

Ah, great -- thanks @jakirkham!

jrbourbeau avatar Aug 05 '22 23:08 jrbourbeau