-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Use docker compose for cassandra integration tests #5520
Conversation
Signed-off-by: Your Name <you@example.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5520 +/- ##
==========================================
- Coverage 96.20% 96.19% -0.02%
==========================================
Files 327 327
Lines 16011 16013 +2
==========================================
Hits 15403 15403
- Misses 434 435 +1
- Partials 174 175 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -53,13 +46,14 @@ apply_schema() { | |||
|
|||
run_integration_test() { | |||
local version=$1 | |||
local major_version=${version%%.*} |
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.
Can we do the same with es/os scripts? Their matrix was changed "7.x" -> "7", which makes the workflow names look weird. I would prefer to go back to 7.x / 8.x
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.
Can be a separate PR for that.
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.
ok i will create a new pr after this
cid=$(docker run "${params[@]}" "${image}:${tag}") | ||
echo "cid=${cid}" >> "$GITHUB_OUTPUT" | ||
echo "${cid}" | ||
local major_version=$1 |
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.
why is this needed?
local schema_version=$2 | ||
local jaegerVersion=$3 | ||
local primaryKeyspace="jaeger_v1_dc1" | ||
local archiveKeyspace="jaeger_v1_dc1_archive" | ||
local compose_file="docker-compose/cassandra/v3.yaml" |
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.
why is this hardcoded to v3?
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.
Thanks!
I would advise you to fix this setting in your git. |
@@ -136,3 +136,10 @@ updates: | |||
allow: | |||
- dependency-name: "bitnami/kafka" | |||
update-types: ["version-update:semver-minor"] | |||
- package-ecosystem: docker | |||
directory: /docker-compose/cassandra |
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.
@hellspawn679 I don't think this is working, the dependabot runs did not pick this up. I suspect that when you pick docker
ecosystem and a directory, dependabot looks for standard file names like Dockerfile
or docker-compose.yml
. Since you're using custom file names like v3.yml
, dependabot doesn't know that they are for docker-compose. We probably need to list the files explicitly instead of specifying a dir only.
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.
Which problem is this PR solving?
Description of the changes
cassaandra-integration-test.sh
to use docker composeHow was this change tested?
bash scripts/cassandra-integration-test.sh 3 v003 v1
bash scripts/cassandra-integration-test.sh 3 v003 v2
bashscripts/cassandra-integration-test.sh 4 v004 v1
scripts/cassandra-integration-test.sh 4 v004 v2
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test