tests: fix scylla_version handling #282
Conversation
| @@ -97,6 +98,12 @@ def get_server_versions(): | |||
| return (cass_version, cql_version) | |||
|
|
|||
|
|
|||
| def get_scylla_version(scylla_ccm_version_string): | |||
| """ get scylla version from ccm before starting a cluster""" | |||
| ccm_repo_cache_dir, _ = ccmlib.scylla_repository.setup(version=scylla_ccm_version_string) | |||
There was a problem hiding this comment.
What actions does setup perform? Because it sounds like it actually creates a new cluster (maybewithout starting it, idk), which is quite a lot of work to just get a version.
Why is this PR necessary? Did CI fail because of this, is there some issue opened?
There was a problem hiding this comment.
setup is making sure the version is downloaded and in place
It's failing at any time you don't use release:5.4
We are testing daily builds in the matrix, it was for quite some time a patch there.
Again the assumption test should only work correctly with released versions only us wrong
There was a problem hiding this comment.
Are the actions performed by setup performed only once when it is called multiple times?
So if we call get_scylla_version and then some other place in the code calls setup will all this work be performed once or twice?
There was a problem hiding this comment.
if the version argument stay the same, it would operate only one.
we can put a lru_cache ontop of it, if you are worried by that.
There was a problem hiding this comment.
If the work is performed only once per argument then it is fine imo, no need to add any cache
|
Only failure is our unstable friend: |
|
@Lorak-mmk meanwhile I see that for tablet and running unstable/master version, the code has change, the PR was to address exactly that use case of using something with isn't a formal release |
|
Can you rebase? Then I'll merge |
test shouldn't assume `SCYLLA_VERSION` is an actual version and should be using ccmlib to read the actual versions strings
fruch
force-pushed
the
fix_scylla_version_handling
branch
from
7530db1 to
6a88ac4
Compare
rebased |
test shouldn't assume
SCYLLA_VERSIONis an actual version and should be using ccmlib to read the actual versions strings