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.

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

ImageConfigParser: Don't sort merged values #170

Merged
merged 1 commit into from
Nov 25, 2024
Merged

ImageConfigParser: Don't sort merged values #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 5 additions & 7 deletions lib/eib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,25 +242,23 @@ def _merge_option(self, section_pattern, option):
opt)
yield (section, opt)
else:
add_vals = Counter()
values = Counter()
for opt in add_opts:
logger.debug('Adding %s %s values from %s', section,
option, opt)
add_vals.update(sect[opt].split())
values.update(sect[opt].split())
yield (section, opt)

del_vals = Counter()
for opt in del_opts:
logger.debug('Removing %s %s values from %s', section,
option, opt)
del_vals.update(sect[opt].split())
values.subtract(sect[opt].split())
yield (section, opt)

# Set the option to the difference of the counters.
# Set the option to the keys that have positive values.
# Merge the values together with newlines like they were
# in the original configuration.
vals = add_vals - del_vals
sect[option] = '\n'.join(sorted(vals.keys()))
sect[option] = '\n'.join(k for k, v in values.items() if v > 0)
Copy link
Member

Choose a reason for hiding this comment

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

I was curious whether Counter provides a shorter way to spell “only keys that have positive values” and it does:

In [7]: values = Counter(foo=3, bar=-1)

In [8]: +values
Out[8]: Counter({'foo': 3})

In [9]: '\n'.join(+values)
Out[9]: 'foo'

So we could write:

Suggested change
sect[option] = '\n'.join(k for k, v in values.items() if v > 0)
sect[option] = '\n'.join(+values)

But I've never seen this before and I think it's too obscure.


def copy(self):
"""Create a new instance from this one"""
Expand Down
8 changes: 4 additions & 4 deletions tests/eib/test_image_config_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ def test_merged_option(config):

assert set(sect) == {'opt'}

# The values will be sorted and newline separated
assert sect['opt'] == 'baz\nfoo'
# The values will be newline separated in the order they appeared.
assert sect['opt'] == 'foo\nbaz'

# Now that the merged option exists, it will override any further
# add/del.
Expand All @@ -129,7 +129,7 @@ def test_merged_option(config):
config.merge()

assert set(sect) == {'opt'}
assert sect['opt'] == 'baz\nfoo'
assert sect['opt'] == 'foo\nbaz'


def test_merged_option_interpolation(config):
Expand Down Expand Up @@ -277,7 +277,7 @@ def test_merged_files(tmp_path, config):
config.merge()

assert set(config['sect']) == {'opt'}
assert config['sect']['opt'] == 'baz\nfoo'
assert config['sect']['opt'] == 'foo\nbaz'
assert set(config['sect-a']) == {'opt'}
assert config['sect-a']['opt'] == ''
assert set(config['sect-b']) == {'opt'}
Expand Down
Loading