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

feat: Add --parallel arg for insights-run #3418

Merged
merged 0 commits into from
Jun 2, 2022
Merged

feat: Add --parallel arg for insights-run #3418

merged 0 commits into from
Jun 2, 2022

Conversation

ryan-blakley
Copy link
Contributor

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

Complete Description of Additions/Changes:

Added parallel execution of rules executed via insights-run. Updated some docstrings that were missing args.

insights/__init__.py Outdated Show resolved Hide resolved
Copy link
Contributor

@xiangce xiangce left a comment

Choose a reason for hiding this comment

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

@ryan-blakley, per my test, I think there is a bug in the following original function:

def generate_incremental(components=None, broker=None):
components = components or COMPONENTS[GROUPS.single]
components = _determine_components(components)
seed_broker = broker or Broker()
for graph in get_subgraphs(components):
broker = Broker(seed_broker)
yield (graph, broker)

It should not generate a new broker when there is an existing one. e.g.

     seed_broker = broker or Broker()
     for graph in get_subgraphs(components):
-        broker = Broker(seed_broker)
-        yield (graph, broker)
+        yield (graph, seed_broker)

@ryan-blakley
Copy link
Contributor Author

@ryan-blakley, per my test, I think there is a bug in the following original function:

def generate_incremental(components=None, broker=None):
components = components or COMPONENTS[GROUPS.single]
components = _determine_components(components)
seed_broker = broker or Broker()
for graph in get_subgraphs(components):
broker = Broker(seed_broker)
yield (graph, broker)

It should not generate a new broker when there is an existing one. e.g.

     seed_broker = broker or Broker()
     for graph in get_subgraphs(components):
-        broker = Broker(seed_broker)
-        yield (graph, broker)
+        yield (graph, seed_broker)

Hmm yeah I've never understood the reasoning behind the seed broker option in the Broker class. I can update that function to remove the seeding.

@xiangce xiangce merged commit 82cfb6d into RedHatInsights:master Jun 2, 2022
xiangce pushed a commit that referenced this pull request Jun 2, 2022
* Added parallel execution of rules excuted via insights-run.
* Updated some docstrings that were missing args.

Signed-off-by: Ryan Blakley <[email protected]>
(cherry picked from commit 82cfb6d)
@ryan-blakley ryan-blakley deleted the add_parallel_rule_execution branch October 21, 2022 13:59
xiangce pushed a commit that referenced this pull request Sep 6, 2024
* Added parallel execution of rules excuted via insights-run.
* Updated some docstrings that were missing args.

Signed-off-by: Ryan Blakley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants