Skip to content

fix: Replace bare except clauses in multicore_utils with specific exceptions#3167

Open
Vaishnav88sk wants to merge 6 commits intoNetflix:masterfrom
Vaishnav88sk:fix/multicore-utils-bare-except
Open

fix: Replace bare except clauses in multicore_utils with specific exceptions#3167
Vaishnav88sk wants to merge 6 commits intoNetflix:masterfrom
Vaishnav88sk:fix/multicore-utils-bare-except

Conversation

@Vaishnav88sk
Copy link
Copy Markdown

Replace bare except: with except ImportError: for the pickle import fallback, and except Exception: in the child process error handler. Bare except clauses catch SystemExit and KeyboardInterrupt which can mask real problems and interfere with process management.

Also expands test coverage from 1 test to 12 tests, covering:

  • Basic parallel_map functionality and order preservation
  • Edge cases (empty input, single element)
  • max_parallel parameter
  • Complex object serialization
  • parallel_imap_unordered behavior
  • Child process failure propagation

PR Type

  • Bug fix
  • New feature
  • Core Runtime change (higher bar -- see CONTRIBUTING.md)
  • Docs / tooling
  • Refactoring

Summary

Replace bare except: clauses with specific exception types and expand test coverage from 1 to 12 tests.

Context / Motivation

Bare except: clauses are a Python anti-pattern (PEP 8, flake8 E722).
In multicore_utils.py:

  1. Line 25: except: for pickle import should be except ImportError:
  2. Line 69: except: in child process should be except Exception:

Bare except clauses catch SystemExit and KeyboardInterrupt which can interfere with process termination and mask real issues.

Issue

Fixes #3164

Changes Made

  • Changed except: to except ImportError: for the cPickle/pickle import
  • Changed except: to except Exception: in the child process handler
  • Expanded test coverage from 1 to 12 tests:
    • Basic parallel_map functionality and order preservation
    • Edge cases (empty input, single element, large datasets)
    • max_parallel parameter behavior
    • Complex object serialization
    • parallel_imap_unordered tests
    • Child process failure propagation

Reproduction

Runtime:

Commands to run:

# paste exact commands

Where evidence shows up:

Before (error / log snippet)
paste here
After (evidence that fix works)
paste here

Root Cause

Why This Fix Is Correct

  • All 12 tests pass
  • Includes test verifying child failures raise MulticoreException

Failure Modes Considered

Tests

  • Unit tests added/updated
  • Reproduction script provided (required for Core Runtime)
  • CI passes
  • If tests are impractical: explain why below and provide manual evidence above

Non-Goals

AI Tool Usage

  • No AI tools were used in this contribution
  • AI tools were used (describe below)
    for formatting

Replace bare except: with except ImportError: for the pickle import
fallback, and except Exception: in the child process error handler.
Bare except clauses catch SystemExit and KeyboardInterrupt which can
mask real problems and interfere with process management.

Also expands test coverage from 1 test to 12 tests, covering:
- Basic parallel_map functionality and order preservation
- Edge cases (empty input, single element)
- max_parallel parameter
- Complex object serialization
- parallel_imap_unordered behavior
- Child process failure propagation
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 29, 2026

Greptile Summary

This PR replaces two bare except: clauses in multicore_utils.py with specific exception types, and expands the test suite from 1 to 12 tests. Both exception-handling fixes are correct and improve the robustness of process management.

  • except: on the cPickle import fallback is narrowed to except ImportError:, which is the only exception that can legitimately occur there.
  • except: in the child process handler is replaced with except BaseException:, preserving the original intent ("no exceptions must escape") while being explicit — traceback.print_exc() and the os._exit() in finally continue to run for all exception types.
  • New tests cover order preservation, empty/single-element inputs, max_parallel, complex object serialization, parallel_imap_unordered variants, and child-failure propagation via MulticoreException.

Confidence Score: 5/5

Safe to merge — the exception-handling changes are minimal, correct, and well-tested.

Both bare-except replacements are straightforward and correct: except ImportError: is the only exception the cPickle import can raise, and except BaseException: in the child process handler preserves the original intent while being explicit. The finally block's os._exit() guarantees the child terminates regardless of exception type. The 11 new tests cover the meaningful behavioral paths including failure propagation.

No files require special attention.

Important Files Changed

Filename Overview
metaflow/multicore_utils.py Replaced bare except: with except ImportError: (pickle import) and except BaseException: (child process handler); both changes are correct and well-motivated.
test/unit/test_multicore_utils.py Expanded from 1 to 12 tests covering order preservation, edge cases, max_parallel, complex objects, parallel_imap_unordered, and child failure propagation; tests look correct.

Reviews (6): Last reviewed commit: "Merge branch 'master' into fix/multicore..." | Re-trigger Greptile

Comment thread metaflow/multicore_utils.py Outdated
Based on greptile bot review: except Exception does not catch
KeyboardInterrupt, SystemExit, or GeneratorExit. Using
BaseException matches the original intent of catching all exceptions
while being explicit (unlike bare except:).
@Vaishnav88sk Vaishnav88sk force-pushed the fix/multicore-utils-bare-except branch from cfecaf5 to d775ddb Compare May 1, 2026 19:39
@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@1c69004). Learn more about missing BASE report.

Files with missing lines Patch % Lines
metaflow/multicore_utils.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #3167   +/-   ##
=========================================
  Coverage          ?   28.15%           
=========================================
  Files             ?      381           
  Lines             ?    52347           
  Branches          ?     9238           
=========================================
  Hits              ?    14738           
  Misses            ?    36674           
  Partials          ?      935           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 5, 2026

Want your agent to iterate on Greptile's feedback? Try greploops.

@Vaishnav88sk
Copy link
Copy Markdown
Author

@romain-intel branch updated!

@saikonen saikonen self-requested a review May 6, 2026 11:26
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.

Replace bare except clauses in multicore_utils with specific exceptions

2 participants