-
Notifications
You must be signed in to change notification settings - Fork 249
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
test_: add multiple status-backend instances #6104
base: develop
Are you sure you want to change the base?
Conversation
Jenkins BuildsClick to see older builds (8)
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #6104 +/- ##
===========================================
- Coverage 60.97% 60.97% -0.01%
===========================================
Files 814 814
Lines 109253 109253
===========================================
- Hits 66619 66615 -4
- Misses 34794 34806 +12
+ Partials 7840 7832 -8
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with some small comments, thank you for this!!
@@ -26,9 +26,12 @@ mkdir -p "${test_results_path}" | |||
all_compose_files="-f ${root_path}/docker-compose.anvil.yml -f ${root_path}/docker-compose.test.status-go.yml" | |||
project_name="status-go-func-tests-$(date +%s)" | |||
|
|||
status_backend_count=10 | |||
export STATUS_BACKEND_URLS=$(eval echo http://${project_name}-status-backend-{1..${status_backend_count}}:3333 | tr ' ' ,) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where should this be used?
@@ -26,9 +26,12 @@ mkdir -p "${test_results_path}" | |||
all_compose_files="-f ${root_path}/docker-compose.anvil.yml -f ${root_path}/docker-compose.test.status-go.yml" | |||
project_name="status-go-func-tests-$(date +%s)" | |||
|
|||
status_backend_count=10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an idea is to have this in an .env file and then take this value from there?
ex
STATUS_BACKEND_COUNT=10
and then in this bash script you just use
source .env
and use the var as $STATUS_BACKEND_COUNT
that way we can also use the same source of truth for python code
url = random.choice(option.status_backend_urls) | ||
except IndexError: | ||
raise Exception("Not enough status-backend containers, please add more") | ||
option.status_backend_urls.remove(url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed it would be nice if we could extract the container id or name here, so we could perform docker exec
commands on it
return response | ||
url = random.choice(option.status_backend_urls) | ||
except IndexError: | ||
raise Exception("Not enough status-backend containers, please add more") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can say here that we can add more container by modifying in the .env file the STATUS_BACKEND_COUNT (if we go with .env file)
action="store", | ||
help="", | ||
default="ws://0.0.0.0:3334", | ||
default=[ | ||
f"http://0.0.0.0:{3314 + i}" for i in range(10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, we could take this from the .env file using https://pypi.org/project/python-dotenv/
@@ -8,4 +8,4 @@ services: | |||
- 8354:8354 | |||
status-backend: | |||
ports: | |||
- 3334:3333 | |||
- 3314-3324:3333 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there some port conflict? as I get errors like :
Error response from daemon: Ports are not available: exposing port TCP 0.0.0.0:3317 -> 0.0.0.0:0: listen tcp 0.0.0.0:3317: bind: Only one usage of each socket address (protocol/network address/port) is normally permitted.
every time when StatusBackend is initialised we pick an uninitialised container
we start 10 instances of status-backend now and it can be easily changed by changing the count