Skip to content

cli: deprecate old config files and plugin dirs #3784

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 9, 2021
Merged

cli: deprecate old config files and plugin dirs #3784

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a16ade
cli: deprecate old config files and plugin dirs
bastimeyer Jun 5, 2021
f2c4081
cli: fix order of config file deprecation log msgs
bastimeyer Jun 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
cli: fix order of config file deprecation log msgs 
- Don't log when loading a config from a deprecated path when using the
  `--config` argument
- Only load the first existing plugin-specific config file
- Keep config file loading order
- Add tests for setup_config_args
  • Loading branch information
@bastimeyer
bastimeyer committed Jun 7, 2021
commit f2c408114d5f64932cf7218a43d27b27f117cba2
31 changes: 20 additions & 11 deletions src/streamlink_cli/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,12 +633,9 @@ def setup_args(parser: argparse.ArgumentParser, config_files: List[Path] = None,
arglist = sys.argv[1:]

# Load arguments from config files
for config_file in filter(lambda path: path.is_file(), config_files or []):
if type(config_file) is DeprecatedPath:
log.info(f"Loaded config from deprecated path, see CLI docs for how to migrate: {config_file}")
arglist.insert(0, f"@{config_file}")
configs = [f"@{config_file}" for config_file in config_files or []]

args, unknown = parser.parse_known_args(arglist)
args, unknown = parser.parse_known_args(configs + arglist)
if unknown and not ignore_unknown:
msg = gettext('unrecognized arguments: %s')
parser.error(msg % ' '.join(unknown))
Expand All @@ -654,20 +651,32 @@ def setup_args(parser: argparse.ArgumentParser, config_files: List[Path] = None,
def setup_config_args(parser, ignore_unknown=False):
config_files = []

if streamlink and args.url:
with ignored(NoPluginError):
plugin = streamlink.resolve_url(args.url)
config_files += [path.with_name(f"{path.name}.{plugin.module}") for path in CONFIG_FILES]

if args.config:
# We want the config specified last to get highest priority
config_files += map(lambda path: Path(path).expanduser(), reversed(args.config))
for config_file in map(lambda path: Path(path).expanduser(), reversed(args.config)):
if config_file.is_file():
config_files.append(config_file)
else:
# Only load first available default config
for config_file in filter(lambda path: path.is_file(), CONFIG_FILES):
if type(config_file) is DeprecatedPath:
log.info(f"Loaded config from deprecated path, see CLI docs for how to migrate: {config_file}")
config_files.append(config_file)
break

if streamlink and args.url:
# Only load first available plugin config
with ignored(NoPluginError):
plugin = streamlink.resolve_url(args.url)
for config_file in CONFIG_FILES:
config_file = config_file.with_name(f"{config_file.name}.{plugin.module}")
if not config_file.is_file():
continue
if type(config_file) is DeprecatedPath:
log.info(f"Loaded plugin config from deprecated path, see CLI docs for how to migrate: {config_file}")
config_files.append(config_file)
break

Comment on lines +667 to +679
Copy link
Member Author

Choose a reason for hiding this comment

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

There is a small change in logic here. Previously it was trying to load multiple plugin specific config files, eg. ~/.config/streamlink/config.twitch and ~/.streamlinkrc.twitch, but since only the first non-plugin-specific default config file gets loaded (see above), plugin configs should have the same logic.

I'm also not sure if we should disable plugin config loading when --config is set or if we should add something like --no-plugin-config for disabling loading plugin configs.

if config_files:
setup_args(parser, config_files, ignore_unknown=ignore_unknown)

Expand Down
Empty file added tests/resources/cli/config/custom
View file
Open in desktop
Empty file.
Empty file added tests/resources/cli/config/primary
View file
Open in desktop
Empty file.
Empty file added tests/resources/cli/config/primary.testplugin
View file
Open in desktop
Empty file.
Empty file added tests/resources/cli/config/secondary
View file
Open in desktop
Empty file.
Empty file added tests/resources/cli/config/secondary.testplugin
View file
Open in desktop
Empty file.
91 changes: 89 additions & 2 deletions tests/test_cli_main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,20 @@
import freezegun

import streamlink_cli.main
import tests.resources
from streamlink.plugin.plugin import Plugin
from streamlink.session import Streamlink
from streamlink_cli.compat import is_win32
from streamlink_cli.compat import DeprecatedPath, is_win32
from streamlink_cli.main import (
NoPluginError,
check_file_output,
create_output,
format_valid_streams,
handle_stream,
handle_url,
log_current_arguments,
resolve_stream_name
resolve_stream_name,
setup_config_args
)
from streamlink_cli.output import FileOutput, PlayerOutput

Expand Down Expand Up @@ -269,6 +272,90 @@ def test_create_output_record_and_other_file_output(self):
console.exit.assert_called_with("Cannot use record options with other file output options.")


@patch("streamlink_cli.main.log")
class TestCLIMainSetupConfigArgs(unittest.TestCase):
configdir = Path(tests.resources.__path__[0], "cli", "config")
parser = Mock()

@classmethod
def subject(cls, config_files, **args):
def resolve_url(name):
if name == "noplugin":
raise NoPluginError()
return Mock(module="testplugin")

session = Mock()
session.resolve_url.side_effect = resolve_url
args.setdefault("url", "testplugin")

with patch("streamlink_cli.main.setup_args") as mock_setup_args, \
patch("streamlink_cli.main.args", **args), \
patch("streamlink_cli.main.streamlink", session), \
patch("streamlink_cli.main.CONFIG_FILES", config_files):
setup_config_args(cls.parser)
return mock_setup_args

def test_no_plugin(self, mock_log):
mock_setup_args = self.subject(
[self.configdir / "primary", DeprecatedPath(self.configdir / "secondary")],
config=None,
url="noplugin"
)
expected = [self.configdir / "primary"]
mock_setup_args.assert_called_once_with(self.parser, expected, ignore_unknown=False)
self.assertEqual(mock_log.info.mock_calls, [])

def test_default_primary(self, mock_log):
mock_setup_args = self.subject(
[self.configdir / "primary", DeprecatedPath(self.configdir / "secondary")],
config=None
)
expected = [self.configdir / "primary", self.configdir / "primary.testplugin"]
mock_setup_args.assert_called_once_with(self.parser, expected, ignore_unknown=False)
self.assertEqual(mock_log.info.mock_calls, [])

def test_default_secondary_deprecated(self, mock_log):
mock_setup_args = self.subject(
[self.configdir / "non-existent", DeprecatedPath(self.configdir / "secondary")],
config=None
)
expected = [self.configdir / "secondary", self.configdir / "secondary.testplugin"]
mock_setup_args.assert_called_once_with(self.parser, expected, ignore_unknown=False)
self.assertEqual(mock_log.info.mock_calls, [
call(f"Loaded config from deprecated path, see CLI docs for how to migrate: {expected[0]}"),
call(f"Loaded plugin config from deprecated path, see CLI docs for how to migrate: {expected[1]}")
])

def test_custom_with_primary_plugin(self, mock_log):
mock_setup_args = self.subject(
[self.configdir / "primary", DeprecatedPath(self.configdir / "secondary")],
config=[str(self.configdir / "custom")]
)
expected = [self.configdir / "custom", self.configdir / "primary.testplugin"]
mock_setup_args.assert_called_once_with(self.parser, expected, ignore_unknown=False)
self.assertEqual(mock_log.info.mock_calls, [])

def test_custom_with_deprecated_plugin(self, mock_log):
mock_setup_args = self.subject(
[self.configdir / "non-existent", DeprecatedPath(self.configdir / "secondary")],
config=[str(self.configdir / "custom")]
)
expected = [self.configdir / "custom", DeprecatedPath(self.configdir / "secondary.testplugin")]
mock_setup_args.assert_called_once_with(self.parser, expected, ignore_unknown=False)
self.assertEqual(mock_log.info.mock_calls, [
call(f"Loaded plugin config from deprecated path, see CLI docs for how to migrate: {expected[1]}")
])

def test_custom_multiple(self, mock_log):
mock_setup_args = self.subject(
[self.configdir / "primary", DeprecatedPath(self.configdir / "secondary")],
config=[str(self.configdir / "non-existent"), str(self.configdir / "primary"), str(self.configdir / "secondary")]
)
expected = [self.configdir / "secondary", self.configdir / "primary", self.configdir / "primary.testplugin"]
mock_setup_args.assert_called_once_with(self.parser, expected, ignore_unknown=False)
self.assertEqual(mock_log.info.mock_calls, [])


class _TestCLIMainLogging(unittest.TestCase):
@classmethod
def subject(cls, argv):
Expand Down