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

docs: adds events and debugging recipe #1246

Merged
merged 5 commits into from Feb 6, 2024

Conversation

big-kahuna-burger
Copy link
Sponsor Contributor

@big-kahuna-burger big-kahuna-burger commented Jan 18, 2024

What it does?

Adds a quick start shell / scaffold for subscribing to all oidc-provider's events with a no-op listeners.

  • Implementers can then just fill in with their custom logic.
  • or they can use this scaffold to (selectively) debug events being emitted.

Why

I think it's useful for library.
Having this repeated manual labor for listeners setup going on with couple of projects, I saved it in a gist.

I see 2 possible issues with this PR from the get go:

  1. Object not being narrow enough when appearing in @params
  2. Is /recipes better instead of adding them here in examples?
    After all they are optional, but still, implementer should consider doing some auditing (at least) so that's why I added them here.

@panva
Copy link
Owner

panva commented Jan 20, 2024

  • It definitely makes sense to move this to a recipe with a corresponding markdown file, not a js one. It should explain how to use this.
  • This recipe should also mention the existing DEBUG=oidc-provider:* namespace, or maybe even use the same one? WDYT?
  • I think that printing the arguments by default is too much, an authentication lifecycle is not going to be readable anyway. WDYT? Is there a way around it?

@big-kahuna-burger
Copy link
Sponsor Contributor Author

big-kahuna-burger commented Jan 20, 2024

  • It definitely makes sense to move this to a recipe with a corresponding markdown file, not a js one. It should explain how to use this.

Sure, will move into .md in recipes.

  • This recipe should also mention the existing DEBUG=oidc-provider:* namespace, or maybe even use the same one? WDYT?

Is oidc-provider:events namespace good?

  • I think that printing the arguments by default is too much, an authentication lifecycle is not going to be readable anyway. WDYT? Is there a way around it?

I think I can recognise universally ctx arg and omit it, because that's biggest readability issue.
I'll play a little bit with that to see what can be done.

Thanks for the feedback.

@big-kahuna-burger
Copy link
Sponsor Contributor Author

Also, I think we can make debug composable into subscribe, so letting it up to implementor to control printing or not and have them use the DEBUG env var to control further what gets printed or not.

@panva
Copy link
Owner

panva commented Jan 22, 2024

  • It definitely makes sense to move this to a recipe with a corresponding markdown file, not a js one. It should explain how to use this.

Sure, will move into .md in recipes.

👍

  • This recipe should also mention the existing DEBUG=oidc-provider:* namespace, or maybe even use the same one? WDYT?

Is oidc-provider:events namespace good?

👍

  • I think that printing the arguments by default is too much, an authentication lifecycle is not going to be readable anyway. WDYT? Is there a way around it?

I think I can recognise universally ctx arg and omit it, because that's biggest readability issue. I'll play a little bit with that to see what can be done.

👍

@panva panva changed the title doc(events): adds events quickstart docs: adds events and debugging recipe Feb 6, 2024
@panva panva merged commit 0bf7696 into panva:main Feb 6, 2024
30 checks passed
@big-kahuna-burger big-kahuna-burger deleted the events-quickstart branch February 6, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants