-
Notifications
You must be signed in to change notification settings - Fork 38
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
update local dev setup docs #1015
Conversation
WalkthroughWalkthroughThe updates focus on improving setup instructions and troubleshooting for both backend and frontend environments, specifically catering to Mac users and those facing database-related issues. Enhancements include detailed guidance on running the system locally, using Docker, and configuring Keycloak. Additionally, there's a shift towards using Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 3
Configuration used: .coderabbit.yaml
Files selected for processing (3)
- README.md (3 hunks)
- docs/Support/Running_Server_Locally.md (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/init.py (1 hunks)
### 4. Mac install issues | ||
Please follow the README.md at the root of the spiff-arena repo first. | ||
If you're on a Mac and trying to run natively (it might translate elsewhere) and get errors when running `poetry install`, many of the issues are related to relational database engine libraries. | ||
You will find lots of people having pain related to these on the internet, so hopefully the solution to your particular problem is easy to find, but this is not always the case. | ||
You may need to install additional system dependencies. | ||
|
||
brew install mysql-client pkg-config | ||
export PKG_CONFIG_PATH="$(brew --prefix)/opt/mysql-client/lib/pkgconfig" | ||
pip install mysqlclient | ||
pip install psycopg2 | ||
|
||
One person decided that mysqlclient and psycopg2 were more trouble than they were worth and removed them from the pyproject.toml, while doing `poetry add pymysql` instead. | ||
If you are using mysql, psycopg2 is not necessary, and pymysql is a pure python implementation of the MySQL client library. | ||
In that case, python won't recognize MySQLdb as a module, so after the above installs (which you would do pre-Poetry), add these lines to __init__.py in the backend project: | ||
|
||
```python | ||
import pymysql; | ||
pymysql.install_as_MySQLdb() | ||
``` | ||
### 5. Access Hosted Version of Spiff |
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.
Lines 41-60 introduce a new section on Mac installation issues, providing a series of steps to address common problems encountered when running poetry install
. While the instructions are helpful, there are a few areas that could be improved for clarity and best practices:
-
Line 47 uses
brew install mysql-client pkg-config
, which is macOS-specific. It's good to specify that these commands are intended for macOS users, as the document addresses Mac installation issues, but a brief mention that these are macOS-specific commands could prevent confusion for users on other platforms. -
The use of
pip install mysqlclient
andpip install psycopg2
(lines 49-50) directly after suggesting the installation of system dependencies implies that these Python packages should be installed globally. It's generally recommended to use virtual environments for Python package management to avoid potential conflicts between project dependencies. Consider suggesting the use ofpoetry add mysqlclient psycopg2
instead, if these packages are indeed necessary for the project, to keep the Python environment consistent and isolated. -
The workaround involving
pymysql
(lines 52-59) is a significant change and could benefit from a more detailed explanation or a link to further documentation. While the instructions are clear, understanding the rationale behind choosingpymysql
overmysqlclient
andpsycopg2
, especially in the context of this project, would be valuable for developers. Additionally, the comment about adding lines to__init__.py
in the backend project could be confusing for newcomers. Specifying the exact location (e.g.,spiffworkflow_backend/__init__.py
) would be helpful.
Consider revising these sections to address the above points, ensuring that the documentation is as clear and helpful as possible for developers setting up the project on their Macs.
## Backend Setup, local | ||
|
||
## If you want to run all locally: | ||
Remember, if you don't need a full-on native dev experience, you can run with docker (see below), which saves you from all the native setup. | ||
If you have issues with the local dev setup, please consult [the troubleshooting guide](https://spiff-arena.readthedocs.io/en/latest/Support/Running_Server_Locally.html). | ||
|
||
If you're on a Mac and trying to run native (might translate elsewhere) running python 3 and get errors with mysqlclient and psycopgen2 when running the Poetry install, you may need to install mysql-client, pkg-config, mysqlclient, and psycopgen2 first, remove mysqlclient and psycopgen2 from the pyproject.toml (or Poetry will try to build them and crash), and run the Poetry install again. | ||
There are three prerequisites for non-docker local development: | ||
|
||
brew install mysql-client pkg-config | ||
export PKG_CONFIG_PATH="$(brew --prefix)/opt/mysql-client/lib/pkgconfig" | ||
pip install mysqlclient | ||
pip install psycopg2 | ||
1. python - [asdf-vm](https://asdf-vm.com) works well for installing this. | ||
2. poetry - `pip install poetry` works | ||
3. mysql - the app also supports postgres. and sqlite, if you are talking local dev). | ||
|
||
Python3 also won't recognize MySQLdb as a module. After the above installs (which you would do pre-Poetry), add these lines to __init__.py in the backend project: | ||
When these are installed, you are ready for: | ||
|
||
import pymysql; | ||
pymysql.install_as_MySQLdb() | ||
cd spiffworkflow-backend | ||
poetry install | ||
./bin/recreate_db clean | ||
./bin/run_server_locally | ||
|
||
On a Mac, port 7000 (used by the app) might be hijacked by Airplay. See the Docker section below. | ||
On a Mac, port 7000 (used by the backend) might be hijacked by Airplay. For those who upgraded to MacOS 12.1 and are running everything locally, your AirPlay receiver may have started on Port 7000 and your server (which uses port 7000 by default) may fail due to this port already being used. You can disable this port in System Preferences > Sharing > AirPlay receiver. |
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.
Lines 12-30 cover the backend setup instructions, emphasizing the option to use Docker for a simpler setup process and providing a step-by-step guide for local development. The instructions are clear but could benefit from a few refinements:
-
Line 21 mentions that the app supports MySQL, PostgreSQL, and SQLite for local development. It would be helpful to provide specific instructions or links to documentation for setting up each of these database systems, as the setup process can vary significantly between them.
-
The steps starting from line 25 (
cd spiffworkflow-backend
) assume that the user has already cloned the repository and is in the correct directory. For clarity and completeness, consider adding a preliminary step instructing users to clone the repository and navigate to the root directory before proceeding with the backend setup. -
The mention of potential port conflicts on Mac (line 30) is valuable information. However, it would be beneficial to provide a solution or workaround for users encountering this issue. For example, instructions on how to change the default port used by the backend or how to disable the AirPlay receiver could help users resolve this conflict more efficiently.
Improving these aspects of the backend setup instructions would make the documentation more comprehensive and user-friendly.
# This is necessary if you want to use use the pymysql library with sqlalchemy rather than mysqlclient. | ||
# This is only potentially needed if you want to run non-docker local dev. | ||
# See the repo's top-level README and the linked troubleshooting guide for details. |
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.
Lines 28-30 comment out the import and use of pymysql
as a replacement for mysqlclient
when using SQLAlchemy. This change is mentioned in the context of local, non-Docker development. However, there are a few considerations:
-
If
pymysql
is recommended for local development, it should be clearly stated why this choice is preferred overmysqlclient
. The benefits and potential drawbacks of usingpymysql
should be documented, either in the code comments or in the project's README. -
Commenting out code in a committed file is generally not a best practice. If this code is necessary for developers opting to use
pymysql
, it should either be included as active code with a conditional check (e.g., based on an environment variable) or removed and provided as part of the documentation. -
The comment mentions looking at the repo's top-level README and a linked troubleshooting guide for details, but it does not provide a direct link or clear instructions on where to find this information. Providing a direct link to the relevant sections of the README or the troubleshooting guide would be more helpful for developers.
Consider revising this section to either include pymysql
setup as part of the active codebase with clear conditions for its use or move these instructions to the documentation with a more detailed explanation.
moves around some of the docs added by tcoz and clarifies a few things.
Summary by CodeRabbit
poetry install
.pymysql
library for local development.