Skip to content

Commit

Permalink
Represent event actions with an enum instead of strings
Browse files Browse the repository at this point in the history
  • Loading branch information
@drehak @vinzenz
drehak authored and vinzenz committed Mar 16, 2020
1 parent b6ad29a commit 3465132
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 48 deletions.
34 changes: 21 additions & 13 deletions repos/system_upgrade/el7toel8/actors/peseventsscanner/libraries/library.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,24 @@
RepositoriesSetupTasks, RpmTransactionTasks, RepositoriesBlacklisted)


Event = namedtuple('Event', ['action', # A string representing an event type (see EVENT_TYPES)
Event = namedtuple('Event', ['action', # An instance of Action
'in_pkgs', # A dictionary with packages in format {<pkg_name>: <repository>}
'out_pkgs', # A dictionary with packages in format {<pkg_name>: <repository>}
'from_release', # A tuple representing a release in format (major, minor)
'to_release', # A tuple representing a release in format (major, minor)
'architectures' # A list of strings representing architectures
])

EVENT_TYPES = ('Present', 'Removed', 'Deprecated', 'Replaced', 'Split', 'Merged', 'Moved', 'Renamed')

class Action(IntEnum):
present = 0
removed = 1
deprecated = 2
replaced = 3
split = 4
merged = 5
moved = 6
renamed = 7


class Task(IntEnum):
Expand Down Expand Up @@ -235,10 +244,9 @@ def parse_entry(entry):

def parse_action(action_id):
"""Get event type name based on PES event's action id"""
if action_id < 0 or action_id >= len(EVENT_TYPES):
if action_id < 0 or action_id >= len(Action):
raise ValueError('Found event with invalid action ID: {}'. format(action_id))

return EVENT_TYPES[action_id]
return Action(action_id)


def parse_packageset(packageset):
Expand All @@ -264,7 +272,7 @@ def parse_architectures(architectures):
def is_event_relevant(event, installed_pkgs, tasks):
"""Determine if event is applicable given the installed packages and tasks planned so far."""
for package in event.in_pkgs.keys():
if package in tasks[Task.remove] and event.action != 'Present':
if package in tasks[Task.remove] and event.action != Action.present:
return False
if package not in installed_pkgs and package not in tasks[Task.install]:
return False
Expand Down Expand Up @@ -298,28 +306,28 @@ def process_events(releases, events, installed_pkgs):

for event in release_events:
if is_event_relevant(event, installed_pkgs, tasks):
if event.action in ('Deprecated', 'Present'):
if event.action in [Action.deprecated, Action.present]:
# Keep these packages to make sure the repo they're in on RHEL 8 gets enabled
add_packages_to_tasks(current, event.in_pkgs, Task.keep)

if event.action == 'Moved':
if event.action == Action.moved:
# Keep these packages to make sure the repo they're in on RHEL 8 gets enabled
# We don't care about the "in_pkgs" as it contains always just one pkg - the same as the "out" pkg
add_packages_to_tasks(current, event.out_pkgs, Task.keep)

if event.action in ('Split', 'Merged', 'Renamed', 'Replaced'):
if event.action in [Action.split, Action.merged, Action.renamed, Action.replaced]:
non_installed_out_pkgs = filter_out_installed_pkgs(event.out_pkgs, installed_pkgs)
add_packages_to_tasks(current, non_installed_out_pkgs, Task.install)
# Keep already installed "out" pkgs to ensure the repo they're in on RHEL 8 gets enabled
installed_out_pkgs = get_installed_event_pkgs(event.out_pkgs, installed_pkgs)
add_packages_to_tasks(current, installed_out_pkgs, Task.keep)

if event.action in ('Split', 'Merged'):
if event.action in [Action.split, Action.merged]:
# Remove those RHEL 7 pkgs that are no longer on RHEL 8
in_pkgs_without_out_pkgs = filter_out_out_pkgs(event.in_pkgs, event.out_pkgs)
add_packages_to_tasks(current, in_pkgs_without_out_pkgs, Task.remove)

if event.action in ('Renamed', 'Replaced', 'Removed'):
if event.action in [Action.renamed, Action.replaced, Action.removed]:
add_packages_to_tasks(current, event.in_pkgs, Task.remove)

do_not_remove = set()
Expand Down Expand Up @@ -498,11 +506,11 @@ def add_output_pkgs_to_transaction_conf(transaction_configuration, events):
message = 'Marking packages for removal:\n'

for event in events:
if event.action in ('Split', 'Merged', 'Replaced', 'Renamed'):
if event.action in (Action.split, Action.merged, Action.replaced, Action.renamed):
if all([pkg in transaction_configuration.to_remove for pkg in event.in_pkgs]):
transaction_configuration.to_remove.extend(event.out_pkgs)
message += '- [{action}] {ins} -> {outs}\n'.format(
action=event.action,
action=event.action.name,
ins=', '.join(sorted(event.in_pkgs.keys())),
outs=', '.join(sorted(event.out_pkgs.keys()))
)
Expand Down
65 changes: 30 additions & 35 deletions repos/system_upgrade/el7toel8/actors/peseventsscanner/tests/unit_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

from leapp.exceptions import StopActorExecution
from leapp.libraries.actor import library
from leapp.libraries.actor.library import (Event,
from leapp.libraries.actor.library import (Action,
Event,
Task,
add_output_pkgs_to_transaction_conf,
filter_out_pkgs_in_blacklisted_repos,
Expand Down Expand Up @@ -42,14 +43,8 @@ def __call__(self):


def test_parse_action(current_actor_context):
assert parse_action(0) == 'Present'
assert parse_action(1) == 'Removed'
assert parse_action(2) == 'Deprecated'
assert parse_action(3) == 'Replaced'
assert parse_action(4) == 'Split'
assert parse_action(5) == 'Merged'
assert parse_action(6) == 'Moved'
assert parse_action(7) == 'Renamed'
for i in range(8):
assert parse_action(i) == Action(i)

with pytest.raises(ValueError):
parse_action(-1)
Expand All @@ -76,7 +71,7 @@ def test_parse_entry(current_actor_context):
{'name': 'split02', 'repository': 'repo'}]}}

event = parse_entry(entry)
assert event.action == 'Split'
assert event.action == Action.split
assert event.in_pkgs == {'original': 'repo'}
assert event.out_pkgs == {'split01': 'repo', 'split02': 'repo'}

Expand All @@ -86,18 +81,18 @@ def test_parse_entry(current_actor_context):
'package': [{'name': 'removed', 'repository': 'repo'}]}}

event = parse_entry(entry)
assert event.action == 'Removed'
assert event.action == Action.removed
assert event.in_pkgs == {'removed': 'repo'}
assert event.out_pkgs == {}


def test_parse_pes_events_file(current_actor_context):
events = parse_pes_events_file('files/tests/sample01.json')
assert len(events) == 2
assert events[0].action == 'Split'
assert events[0].action == Action.split
assert events[0].in_pkgs == {'original': 'repo'}
assert events[0].out_pkgs == {'split01': 'repo', 'split02': 'repo'}
assert events[1].action == 'Removed'
assert events[1].action == Action.removed
assert events[1].in_pkgs == {'removed': 'repo'}
assert events[1].out_pkgs == {}

Expand Down Expand Up @@ -162,8 +157,8 @@ def test_resolve_conflicting_requests(monkeypatch):
monkeypatch.setattr(library, 'filter_out_pkgs_in_blacklisted_repos', lambda x: x)

events = [
Event('Split', {'sip-devel': 'repo'}, {'python3-sip-devel': 'repo', 'sip': 'repo'}, (7, 6), (8, 0), []),
Event('Split', {'sip': 'repo'}, {'python3-pyqt5-sip': 'repo', 'python3-sip': 'repo'}, (7, 6), (8, 0), [])]
Event(Action.split, {'sip-devel': 'repo'}, {'python3-sip-devel': 'repo', 'sip': 'repo'}, (7, 6), (8, 0), []),
Event(Action.split, {'sip': 'repo'}, {'python3-pyqt5-sip': 'repo', 'python3-sip': 'repo'}, (7, 6), (8, 0), [])]
installed_pkgs = {'sip', 'sip-devel'}

tasks = process_events([(8, 0)], events, installed_pkgs)
Expand Down Expand Up @@ -204,16 +199,16 @@ def test_process_events(monkeypatch):
monkeypatch.setattr(library, 'get_repositories_blacklisted', get_repos_blacklisted_mocked(set()))

events = [
Event('Split', {'original': 'rhel7-repo'}, {'split01': 'rhel8-repo', 'split02': 'rhel8-repo'},
Event(Action.split, {'original': 'rhel7-repo'}, {'split01': 'rhel8-repo', 'split02': 'rhel8-repo'},
(7, 6), (8, 0), []),
Event('Removed', {'removed': 'rhel7-repo'}, {}, (7, 6), (8, 0), []),
Event('Present', {'present': 'rhel8-repo'}, {}, (7, 6), (8, 0), []),
Event(Action.removed, {'removed': 'rhel7-repo'}, {}, (7, 6), (8, 0), []),
Event(Action.present, {'present': 'rhel8-repo'}, {}, (7, 6), (8, 0), []),
# this package is present at the start, gets removed and then reintroduced
Event('Removed', {'reintroduced': 'rhel7-repo'}, {}, (7, 6), (8, 0), []),
Event('Present', {'reintroduced': 'rhel8-repo'}, {}, (8, 0), (8, 1), []),
Event(Action.removed, {'reintroduced': 'rhel7-repo'}, {}, (7, 6), (8, 0), []),
Event(Action.present, {'reintroduced': 'rhel8-repo'}, {}, (8, 0), (8, 1), []),
# however, this package was never there
Event('Removed', {'neverthere': 'rhel7-repo'}, {}, (7, 6), (8, 0), []),
Event('Present', {'neverthere': 'rhel8-repo'}, {}, (8, 0), (8, 1), [])]
Event(Action.removed, {'neverthere': 'rhel7-repo'}, {}, (7, 6), (8, 0), []),
Event(Action.present, {'neverthere': 'rhel8-repo'}, {}, (8, 0), (8, 1), [])]
installed_pkgs = {'original', 'removed', 'present', 'reintroduced'}
tasks = process_events([(8, 0), (8, 1)], events, installed_pkgs)

Expand Down Expand Up @@ -251,10 +246,10 @@ def file_not_exists(_filepath):

def test_add_output_pkgs_to_transaction_conf():
events = [
Event('Split', {'split_in': 'repo'}, {'split_out1': 'repo', 'split_out2': 'repo'}, (7, 6), (8, 0), []),
Event('Merged', {'merged_in1': 'repo', 'merged_in2': 'repo'}, {'merged_out': 'repo'}, (7, 6), (8, 0), []),
Event('Renamed', {'renamed_in': 'repo'}, {'renamed_out': 'repo'}, (7, 6), (8, 0), []),
Event('Replaced', {'replaced_in': 'repo'}, {'replaced_out': 'repo'}, (7, 6), (8, 0), []),
Event(Action.split, {'split_in': 'repo'}, {'split_out1': 'repo', 'split_out2': 'repo'}, (7, 6), (8, 0), []),
Event(Action.merged, {'merged_in1': 'repo', 'merged_in2': 'repo'}, {'merged_out': 'repo'}, (7, 6), (8, 0), []),
Event(Action.renamed, {'renamed_in': 'repo'}, {'renamed_out': 'repo'}, (7, 6), (8, 0), []),
Event(Action.replaced, {'replaced_in': 'repo'}, {'replaced_out': 'repo'}, (7, 6), (8, 0), []),
]

conf_empty = RpmTransactionTasks()
Expand Down Expand Up @@ -284,10 +279,10 @@ def test_add_output_pkgs_to_transaction_conf():

def test_filter_events_by_architecture():
events = [
Event('Present', {'pkg1': 'repo'}, {}, (7, 6), (8, 0), ['arch1']),
Event('Present', {'pkg2': 'repo'}, {}, (7, 6), (8, 0), ['arch2', 'arch1', 'arch3']),
Event('Present', {'pkg3': 'repo'}, {}, (7, 6), (8, 0), ['arch2', 'arch3', 'arch4']),
Event('Present', {'pkg4': 'repo'}, {}, (7, 6), (8, 0), [])
Event(Action.present, {'pkg1': 'repo'}, {}, (7, 6), (8, 0), ['arch1']),
Event(Action.present, {'pkg2': 'repo'}, {}, (7, 6), (8, 0), ['arch2', 'arch1', 'arch3']),
Event(Action.present, {'pkg3': 'repo'}, {}, (7, 6), (8, 0), ['arch2', 'arch3', 'arch4']),
Event(Action.present, {'pkg4': 'repo'}, {}, (7, 6), (8, 0), [])
]

filtered = filter_events_by_architecture(events, 'arch1')
Expand All @@ -299,11 +294,11 @@ def test_filter_events_by_architecture():

def test_filter_events_by_releases():
events = [
Event('Present', {'pkg1': 'repo'}, {}, (7, 6), (7, 7), []),
Event('Present', {'pkg2': 'repo'}, {}, (7, 7), (7, 8), []),
Event('Present', {'pkg3': 'repo'}, {}, (7, 8), (8, 0), []),
Event('Present', {'pkg4': 'repo'}, {}, (8, 0), (8, 1), []),
Event('Present', {'pkg5': 'repo'}, {}, (8, 1), (8, 2), [])
Event(Action.present, {'pkg1': 'repo'}, {}, (7, 6), (7, 7), []),
Event(Action.present, {'pkg2': 'repo'}, {}, (7, 7), (7, 8), []),
Event(Action.present, {'pkg3': 'repo'}, {}, (7, 8), (8, 0), []),
Event(Action.present, {'pkg4': 'repo'}, {}, (8, 0), (8, 1), []),
Event(Action.present, {'pkg5': 'repo'}, {}, (8, 1), (8, 2), [])
]

filtered = filter_events_by_releases(events, [(7, 6), (7, 7), (8, 0), (8, 3)])
Expand Down

0 comments on commit 3465132

Please sign in to comment.