-
Notifications
You must be signed in to change notification settings - Fork 2k
Improve error message when database directory is not writable #6294
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -891,9 +891,25 @@ def _open_library(config: confuse.LazyConfig) -> library.Library: | |||||||||||||||||
| lib.get_item(0) # Test database connection. | ||||||||||||||||||
| except (sqlite3.OperationalError, sqlite3.DatabaseError) as db_error: | ||||||||||||||||||
| log.debug("{}", traceback.format_exc()) | ||||||||||||||||||
| # Check for permission-related errors and provide a helpful message | ||||||||||||||||||
| error_str = str(db_error).lower() | ||||||||||||||||||
| dbpath_display = util.displayable_path(dbpath) | ||||||||||||||||||
| if "unable to open" in error_str: | ||||||||||||||||||
|
||||||||||||||||||
| if "unable to open" in error_str: | |
| permission_error_strings = ( | |
| "unable to open", | |
| "readonly", | |
| "read-only", | |
| "attempt to write a readonly database", | |
| ) | |
| if any(msg in error_str for msg in permission_error_strings): |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,42 +12,6 @@ Unreleased | |||||||||||
| New features | ||||||||||||
| ~~~~~~~~~~~~ | ||||||||||||
|
|
||||||||||||
| - **Beets library is now made portable**: item and album-art paths are now | ||||||||||||
| stored relative to the library root in the database while remaining absolute | ||||||||||||
| in the rest of beets. Path queries continue matching both library-relative | ||||||||||||
| paths and absolute paths under the currently configured music directory under | ||||||||||||
| the new storage model. The existing paths in the database are migrated | ||||||||||||
| automatically the first time you run any ``beet`` command after the update. | ||||||||||||
| :bug:`133` | ||||||||||||
|
|
||||||||||||
| .. warning:: | ||||||||||||
|
|
||||||||||||
| make sure you run ``beet version`` (or any other command) at least once | ||||||||||||
| after upgrading to trigger the migration. Only then you can safely move | ||||||||||||
| the library to a new location. | ||||||||||||
|
|
||||||||||||
| Bug fixes | ||||||||||||
| ~~~~~~~~~ | ||||||||||||
|
|
||||||||||||
| - :doc:`plugins/listenbrainz`: Retry listenbrainz requests for temporary | ||||||||||||
| failures. | ||||||||||||
|
|
||||||||||||
| .. | ||||||||||||
| For plugin developers | ||||||||||||
| ~~~~~~~~~~~~~~~~~~~~~ | ||||||||||||
|
|
||||||||||||
| .. | ||||||||||||
| Other changes | ||||||||||||
| ~~~~~~~~~~~~~ | ||||||||||||
|
|
||||||||||||
| 2.9.0 (April 11, 2026) | ||||||||||||
| ---------------------- | ||||||||||||
|
|
||||||||||||
| Beets now officially supports Python 3.14. | ||||||||||||
|
|
||||||||||||
| New features | ||||||||||||
| ~~~~~~~~~~~~ | ||||||||||||
|
|
||||||||||||
| - :ref:`import-cmd` Use ffprobe to recognize format of any import music file | ||||||||||||
| that has no extension. If the file cannot be recognized as a music file, leave | ||||||||||||
| it alone. :bug:`4881` | ||||||||||||
|
|
@@ -70,27 +34,16 @@ New features | |||||||||||
| ``arranger`` fields. Existing libraries are migrated automatically, and | ||||||||||||
| :doc:`plugins/musicbrainz` now preserves each MusicBrainz ``remixer``, | ||||||||||||
| ``lyricist``, ``composer``, and ``arranger`` relation as a separate value. | ||||||||||||
| - :doc:`plugins/musicbrainz`: Store MBIDs for remixers, lyricists, composers, | ||||||||||||
| and arrangers in the new multi-valued fields ``remixers_mbid``, | ||||||||||||
| ``lyricists_mbid``, ``composers_mbid``, and ``arrangers_mbid``. :bug:`5698` | ||||||||||||
| :bug:`5698` | ||||||||||||
| - :doc:`plugins/replaygain`: Conflicting replay gain tags are now removed on | ||||||||||||
| write. RG_* tags are removed when setting R128_* and vice versa. | ||||||||||||
| - :doc:`plugins/fetchart`: Add support for WebP images. | ||||||||||||
| - :doc:`plugins/lastgenre`: Add support for a user-configurable ignorelist to | ||||||||||||
| exclude unwanted or incorrect Last.fm (or existing) genres, either per artist | ||||||||||||
| or globally :bug:`6449` | ||||||||||||
|
|
||||||||||||
| Bug fixes | ||||||||||||
| ~~~~~~~~~ | ||||||||||||
|
|
||||||||||||
| - :doc:`plugins/deezer`: Fix Various Artists albums being tagged with a | ||||||||||||
| localized string instead of the configured ``va_name``. Detection now uses | ||||||||||||
| Deezer's artist ID rather than the artist name string. :bug:`4956` | ||||||||||||
| - :doc:`plugins/listenbrainz`: Paginate through all ListenBrainz listens instead | ||||||||||||
| of fetching only 25, aggregate individual listen events into correct play | ||||||||||||
| counts, use ``recording_mbid`` from the ListenBrainz mapping when available, | ||||||||||||
| and avoid per-listen MusicBrainz API lookups that caused imports to hang on | ||||||||||||
| large listen histories. :bug:`6469` | ||||||||||||
| - Correctly handle semicolon-delimited genre values from externally-tagged | ||||||||||||
| files. :bug:`6450` | ||||||||||||
| - :doc:`plugins/listenbrainz`: Fix ``lbimport`` crashing when ListenBrainz | ||||||||||||
|
|
@@ -114,12 +67,12 @@ Bug fixes | |||||||||||
| switch to the plural field names. :ref:`list-cmd`, and query expressions, | ||||||||||||
| accept the same legacy singular field names and warn users to switch to the | ||||||||||||
| plural field names. :bug:`6483` | ||||||||||||
| - :doc:`plugins/fetchart`: Error when a configured source does not exist or | ||||||||||||
| sources configuration is empty. :bug:`6336` | ||||||||||||
| - :doc:`plugins/rewrite` :doc:`plugins/advancedrewrite`: Fix rewriting | ||||||||||||
| multi-valued fields such as ``genres`` by applying rules to each matching list | ||||||||||||
| entry. Additionally, apply rewrite rules in config order, so that multiple | ||||||||||||
| rules can be applied to the same field. :bug:`6515` | ||||||||||||
| - Improved error message when the database cannot be opened. When SQLite reports | ||||||||||||
| an ``unable to open`` error, beets now suggests checking that the file or | ||||||||||||
| parent directory is writable. The original SQLite error is preserved for | ||||||||||||
| debugging. Also increased the default SQLite busy timeout from 5 s to 30 s to | ||||||||||||
| reduce ``database is locked`` errors during concurrent access, and fixed the | ||||||||||||
| ``cannot not`` typo in the generic database error message. :bug:`1676` | ||||||||||||
|
Comment on lines
+73
to
+75
|
||||||||||||
| debugging. Also increased the default SQLite busy timeout from 5 s to 30 s to | |
| reduce ``database is locked`` errors during concurrent access, and fixed the | |
| ``cannot not`` typo in the generic database error message. :bug:`1676` | |
| debugging, and the ``cannot not`` typo in the generic database error message | |
| is fixed. :bug:`1676` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| [tool.poetry] | ||
| name = "beets" | ||
| version = "2.9.0" | ||
| version = "2.8.0" | ||
| description = "music tagger and library organizer" | ||
| authors = ["Adrian Sampson <adrian@radbox.org>"] | ||
|
Comment on lines
1
to
5
|
||
| maintainers = ["Serene-Arc"] | ||
|
|
@@ -21,7 +21,6 @@ classifiers = [ | |
| "Programming Language :: Python :: 3.11", | ||
| "Programming Language :: Python :: 3.12", | ||
| "Programming Language :: Python :: 3.13", | ||
| "Programming Language :: Python :: 3.14", | ||
| "Programming Language :: Python :: Implementation :: CPython", | ||
| ] | ||
| packages = [ | ||
|
|
@@ -42,7 +41,7 @@ Changelog = "https://github.com/beetbox/beets/blob/master/docs/changelog.rst" | |
| "Bug Tracker" = "https://github.com/beetbox/beets/issues" | ||
|
|
||
| [tool.poetry.dependencies] | ||
| python = ">=3.10,<3.15" | ||
| python = ">=3.10,<4" | ||
|
|
||
|
Comment on lines
43
to
45
|
||
| colorama = { version = "*", markers = "sys_platform == 'win32'" } | ||
| confuse = ">=2.2.0" | ||
|
|
@@ -51,7 +50,7 @@ lap = ">=0.5.12" | |
| mediafile = ">=0.16.0" | ||
| numpy = [ | ||
| { python = "<3.13", version = ">=2.0.2" }, | ||
| { python = ">=3.13", version = ">=2.3.5" }, | ||
| { python = ">=3.13", version = ">=2.3.4" }, | ||
| ] | ||
| packaging = ">=24.0" | ||
| platformdirs = ">=3.5.0" | ||
|
|
@@ -73,12 +72,12 @@ scipy = [ # for librosa | |
| ] | ||
| numba = [ # for librosa | ||
| { python = "<3.13", version = ">=0.60", optional = true }, | ||
| { python = ">=3.13", version = ">=0.63.1", optional = true }, | ||
| { python = ">=3.13", version = ">=0.62.1", optional = true }, | ||
| ] | ||
| mutagen = { version = ">=1.33", optional = true } | ||
| Pillow = { version = "*", optional = true } | ||
| py7zr = { version = "*", optional = true } | ||
| pyacoustid = { version = "^1.3.1", optional = true } | ||
| pyacoustid = { version = "*", optional = true } | ||
| PyGObject = { version = "*", optional = true } | ||
| pylast = { version = "*", optional = true } | ||
| python-mpd2 = { version = ">=0.4.2", optional = true } | ||
|
|
@@ -92,7 +91,7 @@ soco = { version = "*", optional = true } | |
|
|
||
| docutils = { version = ">=0.20.1", optional = true } | ||
| pydata-sphinx-theme = { version = "*", optional = true } | ||
| sphinx = { version = "<9", optional = true } | ||
| sphinx = { version = "*", optional = true } | ||
| sphinx-design = { version = ">=0.6.1", optional = true } | ||
| sphinx-copybutton = { version = ">=0.5.2", optional = true } | ||
| sphinx-toolbox = { version = ">=4.1.0", optional = true } | ||
|
|
@@ -330,6 +329,7 @@ ignore = [ | |
| "beets/**" = ["PT"] | ||
| "test/plugins/test_ftintitle.py" = ["E501"] | ||
| "test/test_util.py" = ["E501"] | ||
| "test/ui/test_ui_init.py" = ["PT"] | ||
| "test/util/test_diff.py" = ["E501"] | ||
| "test/util/test_id_extractors.py" = ["E501"] | ||
| "test/**" = ["RUF001"] # we use Unicode characters in tests | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,11 +16,13 @@ | |
|
|
||
| import os | ||
| import shutil | ||
| import sqlite3 | ||
| import unittest | ||
| from copy import deepcopy | ||
| from random import random | ||
| from unittest import mock | ||
|
|
||
| from beets import config, ui | ||
| from beets import config, library, ui | ||
| from beets.test import _common | ||
| from beets.test.helper import BeetsTestCase, IOMixin | ||
|
|
||
|
|
@@ -119,3 +121,54 @@ def test_create_no(self): | |
| if lib: | ||
| lib._close() | ||
| raise OSError("Parent directories should not be created.") | ||
|
|
||
|
|
||
| class DatabaseErrorTest(BeetsTestCase): | ||
| """Test database error handling with improved error messages.""" | ||
|
|
||
| def test_database_error_with_unable_to_open(self): | ||
| """Test error message when database fails with 'unable to open' error.""" | ||
| test_config = deepcopy(config) | ||
| test_config["library"] = _common.os.fsdecode( | ||
| os.path.join(self.temp_dir, b"test.db") | ||
| ) | ||
|
|
||
| # Mock Library to raise OperationalError with "unable to open" | ||
| with mock.patch.object( | ||
| library, | ||
| "Library", | ||
| side_effect=sqlite3.OperationalError( | ||
| "unable to open database file" | ||
| ), | ||
| ): | ||
|
Comment on lines
+129
to
+143
|
||
| with self.assertRaises(ui.UserError) as cm: | ||
| ui._open_library(test_config) | ||
|
|
||
| error_message = str(cm.exception) | ||
| # Should mention permissions and directory | ||
| self.assertIn("directory", error_message.lower()) | ||
| self.assertIn("writable", error_message.lower()) | ||
| self.assertIn("permissions", error_message.lower()) | ||
|
|
||
| def test_database_error_fallback(self): | ||
| """Test fallback error message for other database errors.""" | ||
| test_config = deepcopy(config) | ||
| test_config["library"] = _common.os.fsdecode( | ||
| os.path.join(self.temp_dir, b"test.db") | ||
| ) | ||
|
|
||
| # Mock Library to raise a different OperationalError | ||
| with mock.patch.object( | ||
| library, | ||
| "Library", | ||
| side_effect=sqlite3.OperationalError("disk I/O error"), | ||
| ): | ||
| with self.assertRaises(ui.UserError) as cm: | ||
| ui._open_library(test_config) | ||
|
|
||
| error_message = str(cm.exception) | ||
| # Should contain the error but not the permissions message | ||
| self.assertIn("could not be opened", error_message) | ||
| self.assertIn("disk I/O error", error_message) | ||
| # Should NOT have the permissions-related message | ||
| self.assertNotIn("permissions", error_message.lower()) | ||
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.
grug see Migration.CHUNK_SIZE removed. but many migrations use self.CHUNK_SIZE (ex MultiValueFieldMigration). now migration run will crash with AttributeError unless every subclass set CHUNK_SIZE. put default back (or move constant into base class those migrations share).