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

scylla_node: populate env variable when running scylla-sstable #501

Closed
wants to merge 3 commits into from

Conversation

tchaikov
Copy link
Contributor

if scylla is built in a dtest environment, and the shared libraries
which it is linked against cannot be found in the testbed's default ld.so.conf
paths, the tests which run scylla-sstable would fail.

so, in this change. let's apply the self._launch_env when
running scylla-sstable as well. previously, these env variables
are only applied when launching scylla as a daemon.

if scylla is built in a dtest environment, and the shared libraries
which it is linked against cannot be found in the testbed's default ld.so.conf
paths, the tests which run scylla-sstable would fail.

so, in this change. let's apply the `self._launch_env` when
running scylla-sstable as well. previously, these env variables
are only applied when launching scylla as a daemon.

Signed-off-by: Kefu Chai <[email protected]>
so we don't need to check for its existence. simpler this way.

Signed-off-by: Kefu Chai <[email protected]>
this change should not make difference. as we generally do not reuse
the same instance of ScyllaNode for launching multiple instances of
daemon, and the mutations applies to self._launch_env does not vary.
but this change should reflect the design better, as
`ScyllaNode.start()` should not mutate `self._launch_env`, which is
used as a template based on which each call of `start()` can customize
it based on the other another env variable.

Signed-off-by: Kefu Chai <[email protected]>
@tchaikov
Copy link
Contributor Author

tested using

$ scripts/dbuild_collect_so.sh ~/dev/scylladb/build/cmake/scylla /tmp/dynamic_libs_for_dtest
$ SCYLLA_DBUILD_SO_DIR=/tmp/dynamic_libs_for_dtest JAVA_HOME=/usr/lib/jvm/jre-11 pytest --cassandra-dir $HOME/dev/scylladb/build/cmake scylla_sstable_test.py::TestScyllaSstableDumpData::test_scylla_sstable_basic

not really a dbuild build. but i also verified by checking the env variables manually.

@tchaikov
Copy link
Contributor Author

https://github.com/scylladb/scylla-ccm/actions/runs/6011490212/job/16304966887?pr=501#step:6:627

i am not sure why s-c failed to connect to scylla ..

@tchaikov
Copy link
Contributor Author

@bhalevy thank you for your review. as this change fails the ccm CI test. see above. i am closing this one in favor of a minimal test: #502 . could you please review #502 instead?

will try to bring the cleanup change back later.

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 this pull request may close these issues.

2 participants