scylla_node: find tool in $install_dir/tools/cqlsh/bin also

[tchaikov]

there are chances that user installs the tool to be found in $PATH, for instance, user might already install the specified tool using pip install scylla-cqlsh. so we should use $PATH as a fallback instead of assuming that they are located under the two candidate directories.

so, in this change, in addition to the candidate directories, we also search for the tool in $PATH, as shutil.which() search the given cmd in $PATH if the specified path parameter is None.

[tchaikov]

as shutil.which() was introduced in Python 3.3. strictly speaking, we should merge this change after #434 is landed.

[fruch]

Can you elaborate on the use case ?

If it's for running dtest straight out of the source tree, we should fix the fallback to take from the correct path in the source tree (and make sure it's built fully)

Using the PATH, might lead to more confusion and use an incorrect version of some of the tools. I think it would cause more harm than it would help...

I'm o.k. that we can define some environment variables for setting the path for looking for tools, if one wants to look there first for some reason.

[tchaikov]

hi @fruch , thanks for your review. sure. i am trying to run dtest right from the build directory of scylladb, but it failed to find cqlsh, so i followed the README of cqlsh and installed it, still no luck. hence this change.

regarding your concern, i updated the patch to use the tools/cqlsh/bin as a fallback, does this make more sense?

and make sure it's built fully

not sure if scylla-ccm can do this. i think it's the user's responsibility.

[fruch]

hi @fruch , thanks for your review. sure. i am trying to run dtest right from the build directory of scylladb, but it failed to find cqlsh, so i followed the README of cqlsh and installed it, still no luck. hence this change.

regarding your concern, i updated the patch to use the tools/cqlsh/bin as a fallback, does this make more sense?

Yes it makes more sense

and make sure it's built fully

not sure if scylla-ccm can do this. i think it's the user's responsibility.

Yes it's the user responsibility, but he doesn't know about it yet :/, we need it documented somewhere.

Anyhow I've fixed cqlsh to be able to work also with older drivers, but it also gonna cause confusion when things starts breaking cause of difference between drivers.

Im not sure if it should be documented in dtest or in scylla core

[tchaikov]

@fruch can we address the documentation of cqlsh in a follow-up PR? or do we need to address it in this very changeset? as i think it's not in the scope of it.

[fruch]

@fruch can we address the documentation of cqlsh in a follow-up PR? or do we need to address it in this very changeset? as i think it's not in the scope of it.

Yes we can