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

Evaluate token source lazily #16

Open
kpritam opened this issue Apr 10, 2020 · 11 comments
Open

Evaluate token source lazily #16

kpritam opened this issue Apr 10, 2020 · 11 comments

Comments

@kpritam
Copy link

kpritam commented Apr 10, 2020

Currently with default settings, sbt fails to load project because it expects GITHUB_TOKEN env variable to be set.

In my opinion, ideally it should get evaluated when it is necessary i.e. at the time of sbt publish

@kpritam
Copy link
Author

kpritam commented Apr 10, 2020

This makes it very hard to load project in any IDE

@djspiewak
Copy link
Owner

I think the problem is that you also need the token in order to run update, meaning that almost anything you do in sbt is going to need it, directly or indirectly. That's why I made it eagerly resolve. I'm open to being convinced that it's worth changing this. :-) Another option is we could add another TokenSource type which is more amenable to IDE stuff, like a system property. You can compose these using ||, which should resolve the issue, at least in theory.

@kpritam
Copy link
Author

kpritam commented Apr 10, 2020

I think the problem is that you also need the token in order to run update, meaning that almost anything you do in sbt is going to need it, directly or indirectly. That's why I made it eagerly resolve. I'm open to being convinced that it's worth changing this. :-) Another option is we could add another TokenSource type which is more amenable to IDE stuff, like a system property. You can compose these using ||, which should resolve the issue, at least in theory.

This plugin can be used in three ways:

  1. Publishing to github package registry
  2. Resolving packages published to registry
  3. Publish + resolve

I understand what you said and I think that applies to second and third case where github token is mandatory even for update.

And this issue is especially for the first case where one can just want to have that github token configured on CI machine from where packages are published. Which means for normal developer workflow this setting is not required.

@djspiewak
Copy link
Owner

I understand what you said and I think that applies to second and third case where github token is mandatory even for update.

And this issue is especially for the first case where one can just want to have that github token configured on CI machine from where packages are published. Which means for normal developer workflow this setting is not required.

Yeah the token is definitely needed even in the second case. So basically the fact that you need to have a token regardless of how you're using the plugin was what led me to make it mandatory.

I think for the normal developer, I would expect their own personal token would be the one configured in their environment somewhere. Obviously, environment variables don't interact particularly well with most IDEs, so adding a different source for the token is probably the right way to go. Like, I don't actually use an IDE, so I don't know what would work best, but almost anything should be supportable. Like, we could have a file on the system where it's read, or we could rely on the git config (e.g. git config --global github.token should work just fine even within an IDE, so long as git is on the PATH).

@mkurz
Copy link
Contributor

mkurz commented Apr 10, 2020

FYI: On my Ubuntu machine I just put

GITHUB_TOKEN="enter_secret_token_here"

in /etc/environment and even IntelliJ picks it up. (Be aware that maybe a system restart is required so this variable takes effect)

@kpritam
Copy link
Author

kpritam commented Apr 10, 2020

I was coming more of from third party users who are not primary authors of code base. They just want to clone, run tests or some tasks to generate some management related reports which does not have to do with publish. More than that they should not misconfigure tokens with write permissions and accidentally publish.

To give an example, my team maintains code base but our client frequently takes a pull and run tests, generate report. Asking them to generate and configure tokens may be little inconvenient.

I personally like to clone third party repos, play with it, make changes and run tests. But if i have to setup github tokens in order to do that seems bit strict unless it is necessary.

@kpritam
Copy link
Author

kpritam commented Apr 10, 2020

FYI: On my Ubuntu machine I just put


GITHUB_TOKEN="enter_secret_token_here"

in /etc/environment and even IntelliJ picks it up. (Be aware that maybe a system restart is required so this variable takes effect)

I am not sure keeping github token as a global env variable on a developer machine would be good idea, may be it is!

@mkurz
Copy link
Contributor

mkurz commented Apr 10, 2020

I am self-employed, I am the only one using the workstation. So I am ok with it.

@djspiewak
Copy link
Owner

I personally like to clone third party repos, play with it, make changes and run tests. But if i have to setup github tokens in order to do that seems bit strict unless it is necessary.

Ahhhhh, you're getting at the case where a project is published to GitHub Packages, but all of its dependencies are on Maven Central. In that case, a token isn't necessary because nothing needs to resolve and no publication will be performed.

This is a valid case to consider, I think. I guess the question would be whether that case is worth complicating the plugin by using a Setting rather than a Task. It's also worth noting that the moment such a project adds a dependency which is itself published to GitHub Packages, the token is once again needed.

@kpritam
Copy link
Author

kpritam commented Apr 11, 2020

This is a valid case to consider, I think. I guess the question would be whether that case is worth complicating the plugin by using a Setting rather than a Task. It's also worth noting that the moment such a project adds a dependency which is itself published to GitHub Packages, the token is once again needed.

Thats correct. I would say if plugin uses Setting instead of Task, that would resolve two issues

  1. No need to configure GITHUB_TOKEN unless it is absolutely necessary
  2. IDE's will work just fine!

I would also add that github packages is at very initial stage at the moment, and not lot of packages available in github registry, hence its possible that most of the projects will just publish there packages to github registry but don't consume anything from there (I bet this will change in the future though!)

My only concern evaluating this property eagerly is that, people who are not familiar with git/sbt will initially struggle or will have to put some efforts to make it work.
But again this investment will be needed if project depends on library which is published to github pkg registry!

@djspiewak
Copy link
Owner

Thats correct. I would say if plugin uses Setting instead of Task, that would resolve two issues

To be clear, we're already using a Setting. What you're asking for is to use a Task. :-)

So the problem with swapping to a Task rather than a Setting is it means you cannot derive Settings from the token. Including, but not limited to, credentials. There are also plugins which derive from this plugin which have complex settings based on the token. So I'm not quite sure exactly how to resolve these issues.

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

No branches or pull requests

3 participants