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

Use read-only SQLite connection #165

Merged
merged 7 commits into from
Jun 5, 2024
Merged

Use read-only SQLite connection #165

merged 7 commits into from
Jun 5, 2024

Conversation

jan-janssen
Copy link
Contributor

When the mendeleev package is located on a Network File System (NFS) Sqlalchemy fails with an OperationalError.

sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) disk I/O error

This is a known limitation of SQLite:

This is because fcntl() file locking is broken on many NFS implementations. You should avoid putting SQLite database files on NFS if multiple processes might try to access the file at the same time.

As the data in the elements.db file never changes, the solution is to connect to the SQLite database using read-only mode. This is implemented in this pull request.

Originally reported by @pkruzikova

@lmmentel
Copy link
Owner

lmmentel commented Jun 5, 2024

Thanks @jan-janssen & @pkruzikova 🙌

In general the changes make sense, however I would rather have a flag that defaults to read-only mode for most cases but still allows to grab the engine for making edits and additions to the db programatically. I might be the only person who's doing that atm but wouldn't want to duplicate the function for the "dev" mode. Could you add a flag to get_engine and get_session to parametrize the access mode?

There seem to be some small linting errors, that should be easy to fix.

@lmmentel lmmentel added the bugfix label Jun 5, 2024
@jan-janssen
Copy link
Contributor Author

Could you add a flag to get_engine and get_session to parametrize the access mode?

@lmmentel I added the flag to get_engine and get_session.

There seem to be some small linting errors, that should be easy to fix.

In addition I fixed the linting errors.

@lmmentel lmmentel self-requested a review June 5, 2024 13:44
@lmmentel lmmentel merged commit 55c2244 into lmmentel:master Jun 5, 2024
20 checks passed
@lmmentel
Copy link
Owner

lmmentel commented Jun 5, 2024

Thanks 👍

@jan-janssen
Copy link
Contributor Author

Perfect, thanks a lot - @lmmentel Can you create a new release?

@lmmentel
Copy link
Owner

lmmentel commented Jun 5, 2024

Yes 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants