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

Allow custom providers in provider() type #74

Merged

Conversation

dw-kihara
Copy link
Contributor

aws_credentials allows users to add custom providers.
However, the aws_credentials_provider:provider() type is currently limited to built-in module names
causing custom providers to be rejected by Dialyzer because their names are not listed in the provider() type.
Example:

C = aws_credentials:make_map(my_provider_module, AccessKeyId, SecretAccessKey, Token),
                          %% ^ This atom is not in the `provider()' type, so Dialyzer rejects it.

This pull request adds the module() type to the provider() type, allowing custom providers to pass Dialyzer checks.
Additionally, I made some minor changes related to custom providers.

@@ -26,7 +26,7 @@ or a map of the shape:

```erlang
#{
provider_source => Provider :: atom(),
provider_source => Provider :: module(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The module() type is effectively the same as the atom() type. However, I think this change clarifies that it represents a module name to the user.

@onno-vos-dev
Copy link
Member

onno-vos-dev commented May 24, 2024

Guessing this is related to your work in: #73 correct? While you added an explicit new type there I'm fine with this change 👌 I would however encourage you to keep the explicit type in the other branch (#73) when you rebase that one 👍

@onno-vos-dev onno-vos-dev merged commit a6d6bf9 into aws-beam:master May 24, 2024
8 checks passed
@dw-kihara
Copy link
Contributor Author

Guessing this is related to your work in: #73 correct?

Yes! I had trouble when writing that provider as a custom provider, so I made this pull request.

I would however encourage you to keep the explicit type in the other branch (#73) when you rebase that one 👍

Agree, I also think the default providers should be listed there.

@dw-kihara dw-kihara deleted the feature/allow_custom_provider_type branch May 24, 2024 01:47
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