Add some troubleshooting notes for the development environment#170
Add some troubleshooting notes for the development environment#170
Conversation
untitaker
left a comment
There was a problem hiding this comment.
i recommend against documenting N workarounds for things that should already just work. if we document it, people will work around the broken automation instead of raising those issues and fixing them.
| a. MacOs note: There are multiple ways to install the python environment on MacOS. | ||
| We tested successfully the installation via `brew`. Other ways, like `pyenv` and | ||
| `uv`, should work as well but we saw incompatibilities with the way `maturin` expects | ||
| environment variables to be set. See below for more details if you want to use | ||
| `pyenv` or `uv` to install python. |
There was a problem hiding this comment.
this should be handled by the envrc and direnv allow. if 3.11 does not exist uv will download it. if that leads to problems let's treat it like any other bug
There was a problem hiding this comment.
if that leads to problems let's treat it like any other bug
We are treating it as any other bug. https://linear.app/getsentry/issue/STREAM-284/automate-all-the-validation-step-in-the-dev-env-guide . Still till that is not fixed people should at least know a way to make it work otherwise nobody would try to fix the issue.
| environment variables to be set. See below for more details if you want to use | ||
| `pyenv` or `uv` to install python. | ||
|
|
||
| 4. Run `make install-dev` from the root of the repo. This will instal `uv` and build |
| Some environment variables are needed to successfully build the library. These are set by direnv. | ||
| They are explained here to give a better understanding in case of issues. | ||
|
|
||
| - `CMAKE_POLICY_VERSION_MINIMUM=3.5`` - This is needed to build librdkafka on | ||
| newer versions of cmake | ||
|
|
||
| Needed environment variables to allow Rust to start a Python interpreter properly: | ||
|
|
||
| - `PYTHONPATH=.` - This is needed in order to make the Python interpreter started by | ||
| Rust find the virtual environment. | ||
|
|
||
| - `PYO3_PYTHON` - This is needed to make the Python interpreter find the right `site-packages` | ||
| directory when started by the Rust code. See `rust-envvars <https://github.com/getsentry/streams/blob/main/scripts/rust-envvars>`_ |
There was a problem hiding this comment.
all of this is set by the envrc
There was a problem hiding this comment.
Yes, this paragraph is not part of the happy path. You should only land here if something went wrong. If something goes wrong knowing what variables are needed for what is needed to allow people to try to fix the issue.
There was a problem hiding this comment.
if this just "neutrally" documents what the envrc does, shouldn't it be possible to document it next to the code that sets those envvars, i.e. as code comments?
There was a problem hiding this comment.
That makes sense to me. I'd move the explanation in the code and reference that from here.
We have a problem to solve. Multiple people needed hours of pairing to get their environment working. The absence of documentation explaining how things worked did not prompt people to fix them but to skip tests. Obviously this is not acceptable. The first (incentive to fix) is both tracked https://linear.app/getsentry/issue/STREAM-284/automate-all-the-validation-step-in-the-dev-env-guide. I doubt we can expect people to fix the environment if the way it works is not clear. |
and now it does, right? i mean most of the envvars you are suggesting in this guide are already set by envrc. i don't think we should suggest to set them explicitly. if they are missing, it's an indication that their direnv setup is broken, and this is the part that should be fixed.
my concern is that we end up with dev environments that drift further away from the golden path. because everybody applied manual fixes on top. |
untitaker
left a comment
There was a problem hiding this comment.
don't want to block this too much. maybe this though: instead of "set this envvar" i'd like to rather suggest "figure out why this envvar isn't being set"
|
|
||
| Needed environment variables to allow Rust to start a Python interpreter properly: | ||
|
|
||
| - `PYTHONPATH=.` - This is needed in order to make the Python interpreter started by |
There was a problem hiding this comment.
this envvar is already set (STREAMS_TEST_PYTHONPATH)
specifically overwriting this envvar like this will fix one bug while re-introducing another. i think if PYTHONPATH needs to be set for cargo test, this points to an issue with direnv. otherwise it's a new bug to be investigated and should be discussed.
also since we now overwrite sys.path within the rust process i'm no longer sure PYTHONPATH has any effect at all
| Some environment variables are needed to successfully build the library. These are set by direnv. | ||
| They are explained here to give a better understanding in case of issues. | ||
|
|
||
| - `CMAKE_POLICY_VERSION_MINIMUM=3.5`` - This is needed to build librdkafka on |
There was a problem hiding this comment.
this is also definitely set by envrc.
Suggesting to set environment variables manually was not my intent. The happy path is only this https://github.com/getsentry/streams/pull/170/files#diff-a5fb029817e5f0c5efdcb0de0545265c5c80aefe309baab2141c676f9c26849eR32-R56. The rest is all about how to understand the process when things go wrong. I'd be happy to clarify better what is the happy path vs what is troubleshooting. What if I explained that explicitly at the beginning of this section https://github.com/getsentry/streams/pull/170/files#diff-a5fb029817e5f0c5efdcb0de0545265c5c80aefe309baab2141c676f9c26849eR32 |
| Some environment variables are needed to successfully build the library. These are set by direnv. | ||
| They are explained here to give a better understanding in case of issues. | ||
|
|
||
| - `CMAKE_POLICY_VERSION_MINIMUM=3.5`` - This is needed to build librdkafka on | ||
| newer versions of cmake | ||
|
|
||
| Needed environment variables to allow Rust to start a Python interpreter properly: | ||
|
|
||
| - `PYTHONPATH=.` - This is needed in order to make the Python interpreter started by | ||
| Rust find the virtual environment. | ||
|
|
||
| - `PYO3_PYTHON` - This is needed to make the Python interpreter find the right `site-packages` | ||
| directory when started by the Rust code. See `rust-envvars <https://github.com/getsentry/streams/blob/main/scripts/rust-envvars>`_ |
There was a problem hiding this comment.
if this just "neutrally" documents what the envrc does, shouldn't it be possible to document it next to the code that sets those envvars, i.e. as code comments?
| environment variables to be set. See below for more details if you want to use | ||
| `pyenv` or `uv` to install python. | ||
|
|
||
| 4. Run `make install-dev` from the root of the repo. This will instal `uv` and build |
There was a problem hiding this comment.
| 4. Run `make install-dev` from the root of the repo. This will instal `uv` and build | |
| 4. Run `make install-dev` from the root of the repo. This will install `uv` and build |
We had a number of issues in managing the development environment.
This collects all the learnings