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

use GenServer instead of Task for the default strategy process #36

Merged
merged 5 commits into from
Sep 14, 2023

Conversation

lovebes
Copy link
Contributor

@lovebes lovebes commented Aug 9, 2022

Polling is a bit more idiomatic when done inside GenServer.
Also, having GenServer sets the setting for dynamic spinning up of strategy modules.
As-is, it can't, because EtsCache needs to have dynamic table naming too, which needs more customization - at that point it should be done in a seperate, custom strategy (probably involving a Registry too).

Therefore this is still "locked" to spin up pre-defined strategy modules.

In addition, I changed the API calls to be synchronous, as you'd still want to process the outcome of the current API call response before you schedule another API call to prevent any possibility of firing the next API call even when processing the current one didn't finish yet.

@victorolinasc
Copy link
Collaborator

This is awesome work @lovebes ! Thanks for your contribution. I should've done this a long time ago.

Just some minor nitpicks with the pipes for opts.

I am interested in having another strategy that spans strategies dynamically. I am also planning on adding more default validations like x5t and so on. If you want to hack on those please do :)

@lovebes
Copy link
Contributor Author

lovebes commented Sep 21, 2022

@victorolinasc I've applied your changes.

Oh awesome! Yes I would very much like to try my hand in creating the strategy that creates other strategies dynamically.
I'll write one up pretty soon! It might help when using it for creating an all-encompassing Auth Service that has to reach out to multiple IDPs via OAuth2, but where the reason behind that is due to multitenancy (each tenant has their own JWKS endpoint).

@lovebes
Copy link
Contributor Author

lovebes commented Sep 25, 2022

@victorolinasc I started working on the dynamic supervisor route, and have realized what more to change in DefaultStrategyTemplate to make it work for both "standalone" and in dynamic supervisor mode. I'll add the changes in this PR, or in another PR if that's better.

@lovebes
Copy link
Contributor Author

lovebes commented Oct 1, 2022

Alright I'm done, I actually changed a lot more and made it useable for when it is needed to be run for dynamically starting strategies. #39 will include and expand this PR.

@victorolinasc victorolinasc self-assigned this Sep 14, 2023
@victorolinasc victorolinasc merged commit 9c34ba4 into joken-elixir:master Sep 14, 2023
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

Successfully merging this pull request may close these issues.

2 participants