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

RefreshTokenStorage schedule documentation #47

Open
epifab opened this issue Oct 16, 2017 · 5 comments
Open

RefreshTokenStorage schedule documentation #47

epifab opened this issue Oct 16, 2017 · 5 comments

Comments

@epifab
Copy link

epifab commented Oct 16, 2017

Hello,
I'm interested in using this project, but there are still some obscure undocumented aspects. The first one is the schedule method in the RefreshTokenStorage. Can you please clarify what is this for and how is it intended to be implemented?
Also the after parameter should probably be a FiniteDuration rather than simply Duration, should it not?
Is there a plan to work on some better documentation in general?
Many thanks

@adamw
Copy link
Member

adamw commented Oct 17, 2017

Hello,

the schedule method is used to schedule an action to be run in the background after the given amount of time. It is used to remove a used token after a (configurable) period of time. The token cannot be removed immediately as there might be multiple concurrent requests and this would cause some of them to fail (race condition).

You are right that after should probalby be a FiniteDuration. Maybe you can submit a PR fixing that?

As for docs, @kijanowski is working on a more docs/faq, see: https://github.com/softwaremill/akka-http-session-faq

@epifab
Copy link
Author

epifab commented Oct 18, 2017

Hello!

Thanks for taking the time to answer.
It's great that someone is working on the docs, I believe this is vital to get more people involved.
Please note that the page you linked is currently 404ing for me.

I understand the reason behind the schedule method now. However, I'm wondering if that could be achieved in a different way, for example by requiring an implicit Scheduler.
My point is that the schedule method is an implementation detail and should probably be transparent to the end user.

@adamw
Copy link
Member

adamw commented Oct 25, 2017

Ah yes, just noticed it's a private temporary repository, we'll be publishing that soon :)

As for schedule... well I'm not very happy about it as well, but I didn't have a better idea at that time :). I suppose in all environments akka will be used anyway, so maybe it would be possible to provide a default implementation that uses the underlying http's actor system?

@epifab
Copy link
Author

epifab commented Oct 27, 2017

Yeah, I believe so. I'm happy to work on a PR to improve this

@adamw
Copy link
Member

adamw commented Oct 30, 2017

@epifab Sounds good, waiting for the code! :)

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

2 participants