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

change: documentation fix bearer.token property #865

Merged

Conversation

matthias-pichler
Copy link
Collaborator

change: documentation

Please specify parts of this PR update:

  • Specification
  • Schema
  • Examples
  • Extensions
  • Use Cases
  • Community
  • CTK
  • Other

Discussion or Issue link:

What this PR does:

fixes the token property inside the bearer authentication being accidentally referred to as bearer as well

Additional information:

Copy link
Member

@cdavernas cdavernas left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@ricardozanini
Copy link
Member

@matthias-pichler-warrify if I may ask, can you add an example to the examples/ directory validating this fix?

@matthias-pichler
Copy link
Collaborator Author

@matthias-pichler-warrify if I may ask, can you add an example to the examples/ directory validating this fix?

I added an example and updated some more typos I found in the Authentication section

change: documentation
Signed-off-by: Matthias Pichler <m.pichler@warrify.com>
Signed-off-by: Matthias Pichler <m.pichler@warrify.com>
Signed-off-by: Matthias Pichler <m.pichler@warrify.com>
Signed-off-by: Matthias Pichler <m.pichler@warrify.com>
Copy link
Member

@cdavernas cdavernas left a comment

Choose a reason for hiding this comment

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

LGTM! Cheers!

@cdavernas cdavernas merged commit 0a48db9 into serverlessworkflow:main Jun 3, 2024
2 of 3 checks passed
@ricardozanini
Copy link
Member

@cdavernas there was error in the CI, we shouldn't have merged it.

@matthias-pichler
Copy link
Collaborator Author

@cdavernas there was error in the CI, we shouldn't have merged it.

You are right: a string with an interpolated variable such as {petId} does not match the uri format ... @ricardozanini should I:

  1. remove the interpolation
  2. update the schema to no longer require the uri format?

@cdavernas
Copy link
Member

@cdavernas there was error in the CI, we shouldn't have merged it.

Damn! I thought it was still the expected "initial" schema bugs we had, sorry guys!

@cdavernas
Copy link
Member

update the schema to no longer require the uri format?

Update the schema so that it supports uri with { } characters. A Regex might do the trick.

@matthias-pichler
Copy link
Collaborator Author

#880

@ricardozanini
Copy link
Member

Just to have it documented, we fixed in #879

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.

4 participants