Skip to content

refactor: centralize migrator filtering to migrators#3897

Merged
beckermr merged 52 commits intomainfrom
filter-split
Apr 3, 2025
Merged

refactor: centralize migrator filtering to migrators#3897
beckermr merged 52 commits intomainfrom
filter-split

Conversation

@beckermr
Copy link
Copy Markdown
Contributor

@beckermr beckermr commented Mar 17, 2025

Description:

This PR centralizes all filtering of the graph done by the migrators to the migrators themselves. This logic was spread across multiple modules and that caused the code to be hard to maintain and unintuitive for folks adding new code. Now every migrator can compute the set of nodes it works on and the set of nodes it has completed without external code.

To do before merging:

  • test pass
  • check that migrators serialize and unserialize properly
  • check that migrator graphs are correct
  • check that status page still works

Checklist:

  • Pydantic model updated or no update needed

Cross-refs, links to issues, etc:

closes #3888

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 18, 2025

Codecov Report

Attention: Patch coverage is 88.03738% with 64 lines in your changes missing coverage. Please review.

Project coverage is 80.49%. Comparing base (df764d1) to head (d81d41d).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
conda_forge_tick/migrators/version.py 66.66% 17 Missing ⚠️
conda_forge_tick/migrators/core.py 85.18% 12 Missing ⚠️
conda_forge_tick/status_report.py 0.00% 11 Missing ⚠️
conda_forge_tick/make_migrators.py 47.36% 10 Missing ⚠️
conda_forge_tick/migrators/broken_rebuild.py 50.00% 4 Missing ⚠️
conda_forge_tick/migrators/migration_yaml.py 96.61% 4 Missing ⚠️
conda_forge_tick/utils.py 40.00% 3 Missing ⚠️
tests/test_migrators.py 89.47% 2 Missing ⚠️
conda_forge_tick/auto_tick.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3897      +/-   ##
==========================================
+ Coverage   78.21%   80.49%   +2.28%     
==========================================
  Files         141      141              
  Lines       15732    15924     +192     
==========================================
+ Hits        12304    12818     +514     
+ Misses       3428     3106     -322     

☔ 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.

@beckermr
Copy link
Copy Markdown
Contributor Author

pre-commit.ci autofix

@beckermr
Copy link
Copy Markdown
Contributor Author

pre-commit.ci autofix

@beckermr
Copy link
Copy Markdown
Contributor Author

The raw number of nodes looks OK, but there are some non-trivial differences that need investigating.

OLD:

  rebuild migration graph sizes:
      aarch64 and ppc64le addition graph size: 2366
      arm osx addition graph size: 2741
      static_lib_migrator graph size: 0
      absl_grpc_proto graph size: 78
      aws_c_common0121 graph size: 13
      aws_c_sdkutils023 graph size: 3
      cfitsio441 graph size: 30
      cfitsio450 graph size: 30
      flang19 graph size: 371
      fmt11 graph size: 54
      geos3131 graph size: 22
      icu75 graph size: 48
      libboost186 graph size: 285
      libflint31 graph size: 19
      numpy2 graph size: 1061
      openmpi5 graph size: 148
      pari217 graph size: 10
      poppler2411 graph size: 15
      proj96 graph size: 30
      python313 graph size: 3242
      python313t graph size: 3243
      pytorch26 graph size: 50
      root_base6344 graph size: 19
      ruby31 graph size: 17
      spdlog114 graph size: 43
      spdlog115 graph size: 43
      xz_to_liblzma_devel graph size: 88

NEW (this PR):

rebuild migration graph sizes:
    aarch64 and ppc64le addition graph size: 2245
    arm osx addition graph size: 2591
    static_lib_migrator graph size: 0
    absl_grpc_proto graph size: 70
    aws_c_common0121 graph size: 12
    aws_c_sdkutils023 graph size: 2
    cfitsio441 graph size: 29
    cfitsio450 graph size: 29
    flang19 graph size: 127
    fmt11 graph size: 49
    geos3131 graph size: 16
    icu75 graph size: 43
    libboost186 graph size: 280
    libflint31 graph size: 17
    numpy2 graph size: 1023
    openmpi5 graph size: 146
    pari217 graph size: 9
    poppler2411 graph size: 13
    proj96 graph size: 27
    python313 graph size: 2998
    python313t graph size: 2999
    pytorch26 graph size: 48
    root_base6344 graph size: 17
    ruby31 graph size: 16
    spdlog114 graph size: 41
    spdlog115 graph size: 41
    xz_to_liblzma_devel graph size: 79

@beckermr
Copy link
Copy Markdown
Contributor Author

beckermr commented Apr 1, 2025

After a fair amount of debugging, here are the main differences, not all of which matter

Version
  nodes
    new - old: []
    old - new: 21828 ['2dfatmic', '4ti2', '7za', '7zip', '_current_repodata_hack', '_go_select', '_libarchive_static_for_cph', '_libgcc_mutex', '_numpy_rc', '_openmp_mutex']...
Version
  effective_nodes
    new - old: ['asio']
    old - new: ['boto3-stubs-lite', 'gnu-units', 'introspection', 'quill', 'ultralytics']
aarch64 and ppc64le addition
  nodes
    new - old: []
    old - new: 72 ['aws-c-compression', 'aws-c-event-stream', 'aws-c-http', 'aws-c-io', 'aws-c-mqtt', 'aws-c-s3', 'aws-c-sdkutils', 'aws-checksums', 'aws-crt-cpp', 'aws-sdk-cpp']...
aarch64 and ppc64le addition
  effective_nodes
    new - old: ['firefox', 'mupdf', 'pymssql', 'sunode']
    old - new: []
arm osx addition
  nodes
    new - old: []
    old - new: 96 ['apache-ant', 'aws-c-compression', 'aws-c-event-stream', 'aws-c-http', 'aws-c-io', 'aws-c-mqtt', 'aws-c-s3', 'aws-c-sdkutils', 'aws-checksums', 'aws-crt-cpp']...
arm osx addition
  effective_nodes
    new - old: ['mupdf']
    old - new: []
flang19
  nodes
    new - old: []
    old - new: 232 ['2dfatmic', 'abinit', 'adani', 'adept', 'adios', 'adios-python', 'aerobulk-python', 'airss', 'ambertools', 'amp-atomistics']...
flang19
  effective_nodes
    new - old: ['mumps']
    old - new: []
libflint32
  effective_nodes
    new - old: []
    old - new: ['conda-forge-pinning']
python313t
  effective_nodes
    new - old: []
    old - new: ['catboost', 'fenics-basix', 'grpc-cpp', 'igwn-segments', 'lal']
static_lib_migrator
  nodes
    new - old: []
    old - new: ['llvmlite']

@beckermr beckermr marked this pull request as ready for review April 3, 2025 16:16
@beckermr beckermr added this pull request to the merge queue Apr 3, 2025
Merged via the queue into main with commit cbbff0d Apr 3, 2025
9 checks passed
@beckermr beckermr deleted the filter-split branch April 3, 2025 16:45
beckermr added a commit that referenced this pull request Apr 3, 2025
beckermr added a commit that referenced this pull request Apr 3, 2025
github-merge-queue bot pushed a commit that referenced this pull request Apr 7, 2025
* Revert "Revert "refactor: centralize migrator filtering to migrators (#3897)"…"

This reverts commit 90ab9c2.

* fix: reduce memory use

* fix: need to filter for pinning graphs

* fix: print size of everything

* fix: try this

* test: mark test as xfail due to expired cert

* fix: add back try finally
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.

separate out migrator filter method into two methods that get combined

1 participant