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

Database parental directories creation if not exist #4327

Merged
merged 12 commits into from
Apr 7, 2022
13 changes: 13 additions & 0 deletions beets/ui/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1206,11 +1206,24 @@ def _configure(options):
util.displayable_path(config.config_dir()))
return config

# Check whether parental directories exist.


def _check_db_directory_exists(path):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe should be called _ensure_db_directory_exists since it does more than just check?

if path == b':memory:': # in memory db
return
newpath = os.path.dirname(path)
if not os.path.isdir(newpath):
from beets.ui.commands import database_dir_creation
if database_dir_creation(newpath):
os.makedirs(newpath)


def _open_library(config):
"""Create a new library instance from the configuration.
"""
dbpath = util.bytestring_path(config['library'].as_filename())
_check_db_directory_exists(dbpath)
try:
lib = library.Library(
dbpath,
Expand Down
10 changes: 10 additions & 0 deletions beets/ui/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -1398,6 +1398,16 @@ def show_version(lib, opts, args):
version_cmd.func = show_version
default_commands.append(version_cmd)

# database_location: return true if user
# wants to create the parent directories.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment looks like it's out of date (it can just be deleted).


def database_dir_creation(path):
# Ask the user for a choice.
return ui.input_yn("The database directory {} does not \
exists, create it (Y/n)?"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
exists, create it (Y/n)?"
exists. Create it (Y/n)?"

So it's two full sentences.

.format(displayable_path(path)))


# modify: Declaratively change metadata.

Expand Down
1 change: 1 addition & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Changelog
Changelog goes here!

New features:
* Create the parental directories for database if they do not exist.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Create the parental directories for database if they do not exist.
* Create the parental directories for database if they do not exist.
:bug:`3808` :bug:`4327`

Including references to where this was discussed.

* :ref:`musicbrainz-config`: a new :ref:`musicbrainz.enabled` option allows disabling
the MusicBrainz metadata source during the autotagging process
Expand Down
3 changes: 2 additions & 1 deletion docs/guides/main.rst
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ place to start::

Change that first path to a directory where you'd like to keep your music. Then,
for ``library``, choose a good place to keep a database file that keeps an index
of your music. (The config's format is `YAML`_. You'll want to configure your
of your music. Beets will prompt you if the parental directories for database do
not exist. (The config's format is `YAML`_. You'll want to configure your
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I think it's probably OK to leave this sentence off? This is an edge case, and the prompt clearly explains to users what's going on, so I don't think we need to warn people that this might happen.

text editor to use spaces, not real tabs, for indentation. Also, ``~`` means
your home directory in these paths, even on Windows.)

Expand Down
1 change: 1 addition & 0 deletions test/test_dbcore.py
Original file line number Diff line number Diff line change
Expand Up @@ -763,5 +763,6 @@ def test_no_results(self):
def suite():
return unittest.TestLoader().loadTestsFromName(__name__)


if __name__ == '__main__':
unittest.main(defaultTest='suite')
40 changes: 38 additions & 2 deletions test/test_ui_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,16 @@
"""Test module for file ui/__init__.py
"""


import os
import shutil
import unittest
from test import _common
from random import random
from copy import deepcopy

from beets import ui
from test import _common
from test.helper import control_stdin
from beets import config


class InputMethodsTest(_common.TestCase):
Expand Down Expand Up @@ -121,8 +126,39 @@ def test_human_seconds(self):
self.assertEqual(h, ui.human_seconds(i))


class ParentalDirCreation(_common.TestCase):
def test_create_yes(self):
non_exist_path = _common.util.py3_path(os.path.join(
self.temp_dir, b'nonexist', str(random()).encode()))
# Deepcopy instead of recovering because exceptions might
# occcur; wish I can use a golang defer here.
test_config = deepcopy(config)
test_config['library'] = non_exist_path
with control_stdin('y'):
ui._open_library(test_config)

def test_create_no(self):
non_exist_path_parent = _common.util.py3_path(
os.path.join(self.temp_dir, b'nonexist'))
non_exist_path = _common.util.py3_path(os.path.join(
non_exist_path_parent.encode(), str(random()).encode()))
test_config = deepcopy(config)
test_config['library'] = non_exist_path

with control_stdin('n'):
try:
ui._open_library(test_config)
except ui.UserError:
if os.path.exists(non_exist_path_parent):
shutil.rmtree(non_exist_path_parent)
raise OSError("Parent directories should not be created.")
else:
raise OSError("Parent directories should not be created.")


def suite():
return unittest.TestLoader().loadTestsFromName(__name__)


if __name__ == '__main__':
unittest.main(defaultTest='suite')