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

Update self hosting docs #6877

Merged
merged 3 commits into from
Aug 8, 2021
Merged

Conversation

chris48s
Copy link
Member

@chris48s chris48s commented Aug 8, 2021

No description provided.

@chris48s chris48s added the documentation Developer and end-user documentation label Aug 8, 2021
## Persistence

To enable Redis-backed GitHub token persistence, point `REDIS_URL` to your
Redis installation.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refs #6854

I think we can probably just remove this note. As I understand it, in order to need/use this, a user would have to set up their own GitHub OAuth app to collect tokens for their pool. I think this situation is so specialised to our instance that we can just remove this note rather than documenting the process for running a token pool. Does this seem reasonable?
All we say about GH_CLIENT_ID and GH_CLIENT_SECRET in the server-secrets docs is "These settings are used by shields.io for GitHub OAuth app authorization but will not be necessary for most self-hosted installations"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, in order to need/use this, a user would have to set up their own GitHub OAuth app to collect tokens for their pool

Isn't the OAuth just a means for soliciting tokens and then filling the pool though? On the off chance a self-hoster did find themselves hitting Hub's 5k hourly limit, I think they'd be able to resolve by getting one or two users (presumably another member of their team) to provide a token which they could stash in Redis

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, but I don't think we provide any alternative way to populate the pool, do we?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if my understanding is off, or if I'm just missing something, but I'm trying to make a distinction between the Shields server loading the tokens from the configured Redis instance, separately from how the tokens get into Redis in the first place.

I know our server also provides the means to accept donated tokens and add those into Redis, but my understanding is that the piece that initializes/loads the tokens is actually agnostic of the mechanism used to insert the tokens to the Redis store. I.e., if I were to manually create a new token and use a script/admin console/whatever to add my token to our Redis instance on Compose, that'd be just like all the other tokens and pulled into the Shields runtime where the server wouldn't care how that token originally got into Redis, wouldn't it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. If you wrote your own script to insert tokens into a redis instance, that would work (I've not actually tested it), but we don't provide one. The pooling code doesn't care how the tokens got there.

Do you think that is a scenario we want to explicitly document for self-hosting users?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think that is a scenario we want to explicitly document for self-hosting users?

I think I'm okay with the proposed changes as-is since it's highly unlikely anyone would need it and the current text can obviously be a source of confusion for users. I just wanted to get clarity on whether the token pool was tightly coupled to the OAuth app approach

@shields-ci
Copy link

shields-ci commented Aug 8, 2021

Messages
📖

Thanks for contributing to our documentation. We ❤️ our documentarians!

📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against d17aa52

@chris48s chris48s merged commit f478180 into badges:master Aug 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Developer and end-user documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants