Skip to content

Commit

Permalink
Merge pull request #249 from minrk/cli-priority
Browse files Browse the repository at this point in the history
Copy self.config in update_config

This protects IPython's CLI priority, which accidentally relies upon self.config being replaced during load_config_files.

This will be backported to 4.2.2

Closes #248.
  • Loading branch information
fperez committed Jun 27, 2016
2 parents 9b10aa9 + 0fcff3c commit 74eef4d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
7 changes: 7 additions & 0 deletions traitlets/config/configurable.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,13 @@ def _config_changed(self, change):

def update_config(self, config):
"""Update config and load the new values"""
# traitlets prior to 4.2 created a copy of self.config in order to trigger change events.
# Some projects (IPython < 5) relied upon one side effect of this,
# that self.config prior to update_config was not modified in-place.
# For backward-compatibility, we must ensure that self.config
# is a new object and not modified in-place,
# but config consumers should not rely on this behavior.
self.config = deepcopy(self.config)
# load config
self._load_config(config)
# merge it into self.config
Expand Down
26 changes: 26 additions & 0 deletions traitlets/config/tests/test_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,32 @@ def test_config_propagation(self):
self.assertEqual(app.foo.i, 10)
self.assertEqual(app.foo.j, 10)
self.assertEqual(app.bar.enabled, False)

def test_ipython_cli_priority(self):
name = 'config.py'
class TestApp(Application):
value = Unicode().tag(config=True)
aliases = {'v': 'TestApp.value'}
app = TestApp()
with TemporaryDirectory() as td:
config_file = pjoin(td, name)
with open(config_file, 'w') as f:
f.write("c.TestApp.value = 'config file'")
# follow IPython's config-loading sequence to ensure CLI priority is preserved
app.parse_command_line(['--v=cli'])
# this is where IPython makes a mistake:
# it assumes app.config will not be modified,
# and storing a reference is storing a copy
cli_config = app.config
assert 'value' in app.config.TestApp
assert app.config.TestApp.value == 'cli'
app.load_config_file(name, path=[td])
assert app.config.TestApp.value == 'config file'
# enforce cl-opts override config file opts:
# this is where IPython makes a mistake: it assumes
# that cl_config is a different object, but it isn't.
app.update_config(cli_config)
assert app.config.TestApp.value == 'cli'

def test_flags(self):
app = MyApp()
Expand Down

0 comments on commit 74eef4d

Please sign in to comment.