-
Notifications
You must be signed in to change notification settings - Fork 16
Description
The documentation on the PYIRONSQLCONNECTIONSTRING env var states
pyiron_base/docs/installation.md
Lines 258 to 260 in 542a8de
| * the `PYIRONSQLTYPE`, `PYIRONSQLFILE`, `PYIRONSQHOST`, `PYIRONSQLDATABASE`, `PYIRONUSER` and `PYIRONSQLUSERKEY` | |
| environment variables define the SQL database connection and can also be summarized in the `PYIRONSQLCONNECTIONSTRING` | |
| environment variable. |
This suggests that you can just set the PYIRONSQLCONNECTIONSTRING and be done with it.
However, setting the PYIRONSQLCONNECTIONSTRING actually has no effect.
Looking at
pyiron_base/pyiron_base/database/manager.py
Lines 83 to 98 in 542a8de
| @property | |
| def sql_connection_string(self) -> str: | |
| sql_type = s.configuration["sql_type"] | |
| if sql_type == "Postgres": | |
| return self._credentialed_sqalchemy_string("postgresql") | |
| elif sql_type == "MySQL": | |
| return self._credentialed_sqalchemy_string("mysql+pymysql") | |
| elif sql_type == "SQLalchemy": | |
| return s.configuration["sql_connection_string"] | |
| elif sql_type == "SQLite": | |
| return "sqlite:///" + s.configuration["sql_file"].replace("\\", "/") | |
| else: | |
| raise ValueError( | |
| f"Invalid SQL type {sql_type} -- This should have been caught at input processing, please contact the " | |
| f"developers" | |
| ) |
we see that it is only considered, if we also change the sql type from the default 'SQlite' to 'SQLalchemy'.
While adapting the documentation would be an option, in my opinion the implementation should be changed.
It is absolutely possible to provide sql connection strings for postgres, mysql as well, so if pyiron gives the user the opportunity to provide a connection string, the natural assumption will be that it works in all those cases.