-
Notifications
You must be signed in to change notification settings - Fork 293
[KFP] Format KFP runs concurrently instead of one by one #8662
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
[KFP] Format KFP runs concurrently instead of one by one #8662
Conversation
…mdelman/mlrun into adamd/add_concurrent_pipeline_format
…oncurrent_pipeline_format
Yacouby
left a comment
There was a problem hiding this 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( |
There was a problem hiding this comment.
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. | ||
| """ |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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
…mdelman/mlrun into adamd/add_concurrent_pipeline_format
📝 Description
Introduced concurrent formatting for runs and aligned tests/mocks with the new flow.
🛠️ Changes Made
Pipelines._format_runs_concurrently(...)usingThreadPoolExecutor(with optional boundedSemaphore) 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
🧪 Testing
experiment_idsare present vs. absent.🔗 References
🚨 Breaking Changes?