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 usage of gen_server? #40

Closed
thalesmg opened this issue May 8, 2024 · 4 comments · Fixed by #43
Closed

Remove usage of gen_server? #40

thalesmg opened this issue May 8, 2024 · 4 comments · Fixed by #43

Comments

@thalesmg
Copy link
Contributor

thalesmg commented May 8, 2024

Hi! Thanks for your work on this library! 🍻

I'm evaluating this library for usage in our project, and I noticed that apparently there's no strict need to use a gen_server for the API calls to Azure. From what I could understand, it appears to me that the only goal of using a gen_server process is to hold the credentials in its state. Please correct me if I'm mistaken. 😅

Would you be willing to accept future PRs that remove the use of gen_server and instead just uses a context map or record to hold the necessary info?

For example:

start(Account, Key) ->
    %% maybe wrap `Key` in a function to avoid printing
    #{account => Account, key => Key, other_stuff => ...}.

With this, there's no more need to serialize all calls through a single process, nor the need to add it to any supervision tree.

And since we are on the acceptable PR topic, would you be willing to accept PRs that format the source code using erlfmt?

@thalesmg
Copy link
Contributor Author

Hi @dkataskin ! Have you had the opportunity to consider this? 😺

@thalesmg
Copy link
Contributor Author

@dkataskin would you be willing to accept PRs that remove this usage?

@dkataskin
Copy link
Owner

@thalesmg Yes, I think it makes sense.

@thalesmg
Copy link
Contributor Author

thalesmg commented Jun 4, 2024

Awesome! I'll try to prepare a couple PR in the coming weeks. Thanks! 🍻

thalesmg added a commit to thalesmg/erlazure that referenced this issue Jun 6, 2024
thalesmg added a commit to thalesmg/erlazure that referenced this issue Jun 6, 2024
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 a pull request may close this issue.

2 participants