Skip to content
This repository was archived by the owner on Feb 14, 2023. It is now read-only.

Move this to the scikit-learn org? #27

Closed
jnothman opened this issue May 9, 2019 · 10 comments · Fixed by scikit-learn/scikit-learn#17921
Closed

Move this to the scikit-learn org? #27

jnothman opened this issue May 9, 2019 · 10 comments · Fixed by scikit-learn/scikit-learn#17921

Comments

@jnothman
Copy link
Collaborator

jnothman commented May 9, 2019

No description provided.

@matthew-brett
Copy link
Contributor

Fine by me. But I'm not a scikit-learn admin.

@rth
Copy link
Contributor

rth commented May 10, 2019

Isn't the initial motivation somewhat similar to that of conda-forge? i.e. that it's easier to maintain all these repos in one place. For instance, adapting the changes/updates of CI config from one repo into others would be more difficult if they are spread in multiple orgs?

Also, there is the question that the CI needs credentials to upload wheels to a rackspace container (that also contains wheels from other projects), and if repos are moved to their respective orgs, that increases the vulnerability surface because more people would have access to it? Of course, the risk is more of stolen credentials than any intentional wrongdoing...

@matthew-brett
Copy link
Contributor

It's probably useful to let me have write access to tthe repo, so I can add fixes when something in the CI framework changes, but you can do that from the scikit-learn org, if y'all take ownership. The credentials are encrypted, so I just have to re-encrypt them, when you move to another repository. Unless someone can break the encryption, they can't get the credentials.

@rth
Copy link
Contributor

rth commented May 10, 2019

The credentials are encrypted, so I just have to re-encrypt them, when you move to another repository.

I meant that since the CI decrypts those to use them, having to write access to this repo is equivalent to having access to the credentials (unless I'm missing something). More people would have write access to it if it is moved to the scikit-learn org (we also currently don't require 2FA from core devs). Though, I agree that it's probably a relatively low risk.

Granted we can restrict access more on the scikit-learn org side, but I'm wondering not so much about this particular case, but more about moving repos out of this org in general..

What does @ogrisel think about it?

@jnothman
Copy link
Collaborator Author

jnothman commented May 12, 2019 via email

@jnothman
Copy link
Collaborator Author

Also, people keep thinking that this repo is about Mac, and I keep forgetting the URL or typing https://github.com/scikit-learn/scikit-learn-wheels into my address bar.

@rth
Copy link
Contributor

rth commented May 15, 2019

I agree that the name of this org is confusing.

My motivation was that we need to duplicate core dev authorization here

Indeed, we also need to do it for conda-forge, but in practice, not all core devs are involved in the final step of building wheels. IMO, because building binary artifacts distributed on PyPi has different security implications than having commit rights on the main git repo, giving access to people who show interest is better than giving it by default to all core devs. Again purely from the perspective of loss of credentials etc.

My main concerns against it are,

  • it will become harder to find related issues by searching in this org. On conda-forge that is the primary way of solving build issues, granted there is less activity and repos here but fragmenting the build repos will not help improve that situation IMO.
  • Matthew has graciously accepted adding fixes on this repo even if it is moved, suggesting that fixes are anticipated (I imagine manylinux2010 builds at some point etc) How about other (potential future contributors) in this org, they would not even know that this repo exists if it is moved.

@rth
Copy link
Contributor

rth commented May 15, 2019

it will become harder to find related issues by searching in this org.

For instance, to give a concrete example Azure builds #23 is something that might interest other projects (numpy, scipy etc.). Currently, it's fairly easy to find that PR by searching in the org, it won't be the case if it's not part of this org.

@ogrisel
Copy link
Contributor

ogrisel commented May 15, 2019

I am fine with the change as well.

@rth
Copy link
Contributor

rth commented May 15, 2019

Feel free to move it then.

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

Successfully merging a pull request may close this issue.

4 participants