-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 docs following token pool changes #8933
Conversation
|
| shields-io-production | Access management | @calebcartwright, @chris48s, @paulmelnikow | | ||
| Compose.io Redis | Account owner | @paulmelnikow | | ||
| Compose.io Redis | Account access | @paulmelnikow | | ||
| Compose.io Redis | Database connection credentials | @calebcartwright, @chris48s, @paulmelnikow, @pyvesb | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify what changed here (prettier reformatted the whole table so it is hard to tell), the only change here is I removed the 3 rows about compose.io.
There might be some other things in this table that could do with review but that can be another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only minor items noted inline so will go ahead and approve in case you want to press on. Happy to re-:+1: though if you want to make any adjustments
|
||
To run the integration tests: | ||
|
||
- You must have Redis installed and in your PATH. Use `brew install redis`, `apt-get install redis`, etc. The test runner will start the server automatically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it would be worth going ahead and nuking this? Understand it's accurate atm, but I think that's likely to change fairly quickly (guessing your plan is to delete this when we drop the redis portion of the codebase?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it can be removed when I do #8932
For the moment, if you try to run the integration tests without redis they won't work so I think it is worth keeping it in the docs for the moment
2. The server keeps the [resource cache][] in memory. It is neither | ||
persisted nor inspectable. | ||
|
||
[github auth admin endpoint]: https://github.com/badges/shields/blob/master/services/github/auth/admin.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha! Seems like there's been several tentacles from this still lingering around
Co-authored-by: Caleb Cartwright <[email protected]>
Did a quick skim over all the docs relating to token pooling/redis/postgres and made some updates to reflect the current state of play. Tbh I probably should have done some of this in #8922 but I didn't.. so I'm doing it now.