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

Improve API_IPS configuration process #33

Open
xendarboh opened this issue Sep 14, 2022 · 4 comments
Open

Improve API_IPS configuration process #33

xendarboh opened this issue Sep 14, 2022 · 4 comments

Comments

@xendarboh
Copy link

DatabaseAPI IPs are read directly from the .env file, creating challenges within a docker container context as the docker image must be built with the configuration.

The latest searchseco/controller Docker Image on Docker Hub currently has an error within it's .env file:

docker pull searchseco/controller
docker run -ti --rm --entrypoint /bin/bash searchseco/controller
root@cea3c05c0d8b:/# cat /controller/build/.env 
API_IPS=API_IPS=131.211.31.176?8003,131.211.31.179?8003,131.211.31.183?8003

So running the container fails:

docker run --rm --name controller-container -e github_token=xxxxx -e worker_name=xxxxx searchseco/controller
2022-09-14 06:55:01.644 (   0.006s) [controller      ]           commands.cpp:516   INFO| Starting a worker node with 4 cpu cores
2022-09-14 06:55:01.644 (   0.006s) [controller      ]         networking.cpp:196    ERR| E019 - No IPs found in .env file. IPs should be in format API_IPS=ip1?port1,ip2?port2

It would be useful to be able to set the API_IPS at runtime, for example:

docker run --rm --name controller-container -e github_token=xxxxx -e worker_name=xxxxx -e "API_IPS=138.201.216.223?8003,95.217.155.82?8003" searchseco/controller

A potential solution could use a bash script as the Dockerfile ENTRYPOINT to intercept configuration from the environment and write it to the .env file before calling searchseco start.

@slingerbv
Copy link
Contributor

First and foremost: Thank you! We were looking at this bug for a long time and couldn't figure it out. It's looking like it's fixed now.

But why would we make the IPs scriptable? Our fear has always been that someone starts inefficiently running their own DB cluster, making our work moot ;-).

@xendarboh
Copy link
Author

I'm still getting to know the project to suggest solutions more supportive of the intentions.

As it is now, for a developer (or whoever) to build and use the docker image themselves, they must configure API_IPS so that implies it should be configured. And they have to figure out what values to use for API_IPS to connect to the official DB... I ran and extracted them from the published image, messy. So, perhaps an improvement would be to more completely remove it from awareness of others; either include it in the .env file under git control and remove it as a required step within the README, or update README to explain what value to use there, or something else. ?

As open source, humans will do what humans will do and maybe they don't have an incentive to run their own DB cluster unless the official one itself was deemed too inefficient, or too centralized, etc. Seems most would want to connect to the official DB so this issue could be about making that more effortless to do by default.

@xendarboh xendarboh changed the title Accept API_IPS configuration as an environment variable Immprove API_IPS configuration process Sep 14, 2022
@slingerbv
Copy link
Contributor

Wait, wouldn't this be very easy? Shall I put a bounty on it?

@xendarboh xendarboh changed the title Immprove API_IPS configuration process Improve API_IPS configuration process Oct 6, 2022
@slingerbv
Copy link
Contributor

So... It's already implemented or should I put a bounty on it?

@ghost ghost self-assigned this Oct 29, 2022
@ghost ghost added enhancement New feature or request rework and removed enhancement New feature or request labels Oct 29, 2022
@ghost ghost added this to the Project Reboot milestone Nov 5, 2022
@ghost ghost removed their assignment Nov 5, 2022
@ghost ghost removed this from the void milestone Nov 5, 2022
@ghost ghost added the flaw label Nov 8, 2022
@ghost ghost removed flaw labels Jan 19, 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

No branches or pull requests

2 participants