Skip to content

Conversation

@adamdelman
Copy link
Contributor

@adamdelman adamdelman commented Sep 29, 2025

📝 Description

Introduced concurrent formatting for runs and aligned tests/mocks with the new flow.


🛠️ Changes Made

  • Concurrent formatting pipeline
    • Added Pipelines._format_runs_concurrently(...) using ThreadPoolExecutor (with optional bounded Semaphore) to format runs in parallel while preserving discovery order and skipping per-run failures with a warning.
    • list_pipelines(...) now invokes the concurrent formatter for each streamed page.

✅ Checklist

  • I have tested the changes in this PR

🧪 Testing

  • Verified parallel formatting behavior returns correct data.
  • Confirmed error enrichment happens only for unsuccessful statuses.
  • Asserted filter behavior when experiment_ids are present vs. absent.
  • Updated/ran unit tests that exercise the new concurrent formatter and low-level KFP list runs mocks.

🔗 References


🚨 Breaking Changes?

  • No

@adamdelman adamdelman requested review from a team, liranbg and quaark as code owners September 29, 2025 06:00
@adamdelman adamdelman changed the title Adamd/add concurrent pipeline format [KFP] Format KFP runs concurrently instead of one by one Sep 29, 2025
Copy link
Member

@Yacouby Yacouby left a comment

Choose a reason for hiding this comment

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

some suggestions

try:
formatted_runs.append(future.result())
except Exception:
mlrun.utils.logger.warning(
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be error and not warning ?

run=run,
"""
Submit formatting tasks concurrently and emit results in discovery order.
"""
Copy link
Member

Choose a reason for hiding this comment

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

worth to add a check at the beginning if runs is empty then return

self._format_run(
run=run,
"""
Submit formatting tasks concurrently and emit results in discovery order.
Copy link
Member

Choose a reason for hiding this comment

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

add more doc explaining the use of the semaphore here and the max_workers

@adamdelman adamdelman requested a review from Yacouby December 1, 2025 11:48
…mdelman/mlrun into adamd/add_concurrent_pipeline_format
@liranbg liranbg merged commit fb1756b into mlrun:development Dec 2, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants