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

Compile -> Runtime Configuration ? #44

Closed
asmodehn opened this issue May 18, 2021 · 5 comments · Fixed by #106
Closed

Compile -> Runtime Configuration ? #44

asmodehn opened this issue May 18, 2021 · 5 comments · Fixed by #106

Comments

@asmodehn
Copy link
Contributor

Hi everyone, I am opening this issue to start a conversation regarding compiled / runtime configuration.

Few reasons for this :

  • I wanted to test the behaviour of the code when the endpoint is not reachable, with bypass, but the endpoint is currently set at compile time, and cannot be changed while tests are running.
  • I wanted to test authenticated/anonymous requests, but the apikey/secret is currently set at compile time, and, just like the endpoint, cannot be changed at runtime.
  • I want to have my apikey/secret in a file in user's home, not in a config.exs (danger of accidental commit). I plan to use vapor to configure processes on application startup.
  • I want to use binance.ex in a multiprocess app...
  1. adding a private function to the module where we call Application.get_env(), instead of a module tag would make it changeable at runtime. Although not practical it would work fine in monoprocess situations (only one configuration possible) and would allow testing different configurations.

  2. Would be better: functions need more parameters for tests (or user code) to be able to change endpoint/apikey/secret on call which will make this code usable in a multiprocess situation (one process can connect to one account, another to a different account for instance, they would crash independently, etc.).

Regarding 2., in my specific usecase, I am currently thinking to have one process for public requests, another one for authenticated requests (only this process would be started with apikey and secret). Call rates will be different, public requests can probably be cached for a short time (don't modify the real world), authenticated clients can crash and respawn independently, etc.

Adding these parameters to functions can make the API more complex, so we should think about how to do this without degrading the dev experience...

I see a couple of options:

  • pass a datastructure to all api functions, such as :
%Binance.Connection{ endpoint: _, apikey: _ , secret: _ }
  • have an agent holding that "connection" data, configurable at runtime, and providing the same api.
  • there are probably other options, but I am not sure which one would be "simple and intuitive enough" for devs here. Python async http clients have "session" concept that may also be useful here: https://docs.aiohttp.org/en/stable/#client-example

What do you think about 1. ? I can do a quick PR, if this is something you are interested in.

What do you think about 2. ?

Also, let me know of any other issue that would be useful that would be useful to work on and fall into this "configuration" topic. I am currently working on this on my code, so I can probably contribute something similar here.

Cheers !

@asmodehn
Copy link
Contributor Author

asmodehn commented Jun 2, 2021

Regarding this topic, I recently encountered Krakex which allows passing a Client struct to the API functions (see 'Usage' in the README).
This allows using the lib with multiple accounts, changing endpoint in tests, and in general provide more usage flexibility.

@asmodehn
Copy link
Contributor Author

If anyone is interested to discuss about this, I made the changes in a branch on my fork : asmodehn#1

@davidcroda
Copy link

Just commenting to note that I was recently bit by the compile time pinning of these config values. It is very non intuitive and extremely frustrating to troubleshoot. At a minimum there should be a warning or a note in a comment in the sample config mentioning that if you change the values after the first run you will need to run mix deps.compile binance --force for them to update.

dvcrn added a commit that referenced this issue Oct 29, 2023
Endpoint was getting baked in at compile-time, so changing env at
runtime had no effect.

Fixes #44
dvcrn added a commit that referenced this issue Oct 29, 2023
Endpoint was getting baked in at compile-time, so changing env at
runtime had no effect.

Fixes #44
@dvcrn
Copy link
Owner

dvcrn commented Oct 29, 2023

Sorry for not catching this earlier. Was a simple one-line change that's causing this.

I've changed it to use Application.get_env which means Application.set_env can just change it again. Next step is to pass those in dynamically, but for now this should at least resolve that issue

@dvcrn
Copy link
Owner

dvcrn commented Oct 30, 2023

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.

3 participants