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

Remove pointer map used to get AuthProperties #87

Conversation

calvinmclean
Copy link
Contributor

What this PR does / why we need it:
Fixes #86

See issue for more details, but basically the pointer map used to get AuthProperties based on Scheme caused all parsed Workflows to reference the same property which does not allow unique tokens.

Special notes for reviewers:

Additional information (if needed):

Fixes serverlessworkflow#86

Signed-off-by: Calvin McLean <calvinlmc@gmail.com>
Copy link
Member

@spolti spolti left a comment

Choose a reason for hiding this comment

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

Hey, you was fast, thanks for this.

@ricardozanini ricardozanini removed the request for review from tsurdilo October 3, 2022 15:19
Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

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

Thank you!! Just let a small comment enhance the test case; other than that seems pretty good. Many thanks for your contribution.

model/auth_test.go Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@ddce579). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #87   +/-   ##
=======================================
  Coverage        ?   16.52%           
=======================================
  Files           ?       14           
  Lines           ?      575           
  Branches        ?        0           
=======================================
  Hits            ?       95           
  Misses          ?      460           
  Partials        ?       20           
Flag Coverage Δ
sdk-go 16.52% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Calvin McLean <calvinlmc@gmail.com>
@ricardozanini ricardozanini merged commit b4eb66f into serverlessworkflow:main Oct 3, 2022
@calvinmclean
Copy link
Contributor Author

@ricardozanini would it be possible to get a new release version with this change?

@ricardozanini
Copy link
Member

@calvinmclean we are currently implementing version 0.8 of the spec, so it's not a good moment for a release right now. Due to the nature of Go, can't you use the commit ID in go.mod instead?

@calvinmclean calvinmclean deleted the fix/86/authproperties-pointers branch January 25, 2023 02:05
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.

map of AuthProperties pointers causes all parsed Workflows to have the same AuthProperties
4 participants