You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Following up from #1664: We should exit gracefully with a useful error message if we get an OperationalError from SQLite that indicates a permissions problem on write.
Hello,
I am interested in contributing to this issue. I've already add some lines of code in file library.py but i don't know if this is what you want. Please explain me a little more this issue. Thank you a lot. The code is below: def __str__(self): if os.access(self.path,os.W_OK): return u'The file is not writable. Permissions problem is detected ' + super(WriteError, self).text() else: return u'error writing ' + super(WriteError, self).text()
Hi! You can format the code using a single pair of ``` delimiters. See GitHub's Markdown help for details.
I don't quite understand what you're proposing here. The linked issue shows an exception being raised from a connection.execute call; we'll need to handle that exception. It looks like you might be looking at the code that's used to write music files themselves? But the issue at hand is about accessing the SQLite database.
Hi! You can format the code using a single pair of ``` delimiters. See
GitHub's Markdown help for details.
I don't quite understand what you're proposing here. The linked issue
shows an exception being raised from a connection.execute call; we'll
need to handle that exception. It looks like you might be looking at the
code that's used to write *music files* themselves? But the issue at hand
is about accessing the SQLite database.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1676 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AOp3kvpu3hJ402TpGIXpqmjniVXf_iE-ks5rqx5lgaJpZM4GZQIU>
.
def mutate(self, statement, subvals=()):
"""Execute an SQL statement with substitution values and return
the row ID of the last affected row.
"""
try:
cursor = self.db._connection().execute(statement, subvals)
return cursor.lastrowid
except sqlite3.OperationalError,e:
print ('Problem with database connection:%s' str(e))
Thanks! This is indeed where the error occurs. There are a few more things to address:
Just printing an error message won't cut it—we need to stop beets entirely if this happens. That usually happens by raising a UserError, but that's usually best done in UI-related code. So, we should probably catch this error in one of the "main" functions in beets.ui instead of here.
An OperationalError can be raised in many different situations. We need to specifically detect when it indicates a permissions problem.
For sensitive, central parts of the code like this, we really need tests to show that we've fixed the issue. Can you please start by crafting a unit test that triggers the problem so you can be sure that the code you add actually fixes it?
The modern syntax for handling exceptions is except sqlite3.OperationalError as e:.
Hello,
Could you please help me? I change the permissions to my library but the command os.access(path,os.W_OK) returns true. Is there another way to check it?
I've made the unittest also.
Yes, checking the permissions ahead of time with os.stat and similar functions is probably not the right way to go. Instead, we'll want to let the SQLite module produce an exception, and then inspect that exception value to see what the explanation is.
The e.args[0] gives the message unable to open the database file and the traceback too. So, how can i understand that this message indicates permission problem?
That may be all the detail we can extract—in which case we'll just need to report that we were unable to open the database file, which might indicate a permissions problem.
Was looking for a 'good first issue' ... this thread is old but with no apparent resolution so I went digging ... didn't take long for me to discover the following contained in ui/__init__.py:
def _open_library(config):
"""Create a new library instance from the configuration.
"""
dbpath = util.bytestring_path(config['library'].as_filename())
try:
lib = library.Library(
dbpath,
config['directory'].as_filename(),
get_path_formats(),
get_replacements(),
)
lib.get_item(0) # Test database connection.
except (sqlite3.OperationalError, sqlite3.DatabaseError) as db_error:
log.debug(u'{}', traceback.format_exc())
raise UserError(u"database file {0} cannot not be opened: {1}".format(
util.displayable_path(dbpath),
db_error
))
log.debug(u'library database: {0}\n'
u'library directory: {1}',
util.displayable_path(lib.path),
util.displayable_path(lib.directory))
return lib
...specifically the exception statement here suggests that the issue has been resolved?!
Hmm, good question. It would be worth investigating a little further to see how SQLite actually fails when the directory is read-only. From the original thread, for example, it seems possible that the error isn't raised until we actually try to write to the database—which would mean that this handler on the call to open the database wouldn't catch it.
Activity
beetbox#1676 Add a more descriptive error message
Mary011196 commentedon Mar 29, 2017
Hello,
I am interested in contributing to this issue. I've already add some lines of code in file library.py but i don't know if this is what you want. Please explain me a little more this issue. Thank you a lot. The code is below:
def __str__(self):
if os.access(self.path,os.W_OK):
return u'The file is not writable. Permissions problem is detected ' + super(WriteError, self).text()
else:
return u'error writing ' + super(WriteError, self).text()
sampsyo commentedon Mar 30, 2017
Hi! You can format the code using a single pair of ``` delimiters. See GitHub's Markdown help for details.
I don't quite understand what you're proposing here. The linked issue shows an exception being raised from a
connection.execute
call; we'll need to handle that exception. It looks like you might be looking at the code that's used to write music files themselves? But the issue at hand is about accessing the SQLite database.Mary011196 commentedon Mar 30, 2017
Mary011196 commentedon Mar 30, 2017
I thought to add this in db.py. Is this right?
Thank you
sampsyo commentedon Mar 30, 2017
Thanks! This is indeed where the error occurs. There are a few more things to address:
UserError
, but that's usually best done in UI-related code. So, we should probably catch this error in one of the "main" functions inbeets.ui
instead of here.except sqlite3.OperationalError as e:
.Mary011196 commentedon Apr 8, 2017
Hello,
Could you please help me? I change the permissions to my library but the command
os.access(path,os.W_OK)
returns true. Is there another way to check it?I've made the unittest also.
Thank you in advance
sampsyo commentedon Apr 9, 2017
Yes, checking the permissions ahead of time with
os.stat
and similar functions is probably not the right way to go. Instead, we'll want to let the SQLite module produce an exception, and then inspect that exception value to see what the explanation is.As far as I can tell, there's no way to get a numerical error code out of an SQLite exception:
https://bugs.python.org/issue16379
https://bugs.python.org/issue24139
So the thing to do will be to use
e.args[0]
to get the error message and check whether that equals the error string referenced in the traceback above.Mary011196 commentedon Apr 10, 2017
The
e.args[0]
gives the messageunable to open the database file
and the traceback too. So, how can i understand that this message indicates permission problem?Thank you
sampsyo commentedon Apr 10, 2017
That may be all the detail we can extract—in which case we'll just need to report that we were unable to open the database file, which might indicate a permissions problem.
apitofme commentedon Aug 17, 2019
Was looking for a 'good first issue' ... this thread is old but with no apparent resolution so I went digging ... didn't take long for me to discover the following contained in
ui/__init__.py
:...specifically the exception statement here suggests that the issue has been resolved?!
sampsyo commentedon Aug 17, 2019
Hmm, good question. It would be worth investigating a little further to see how SQLite actually fails when the directory is read-only. From the original thread, for example, it seems possible that the error isn't raised until we actually try to write to the database—which would mean that this handler on the call to open the database wouldn't catch it.