Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Pipelines] Handle missing custom ARCHITECTURES list - #6778

#6778
Draft
mattfaltyn wants to merge 1 commit into
modular:mainmodular/modular:mainfrom
mattfaltyn:agent/handle-missing-custom-architecturesmattfaltyn/modular:agent/handle-missing-custom-architecturesCopy head branch name to clipboard
Draft

[Pipelines] Handle missing custom ARCHITECTURES list#6778
mattfaltyn wants to merge 1 commit into
modular:mainmodular/modular:mainfrom
mattfaltyn:agent/handle-missing-custom-architecturesmattfaltyn/modular:agent/handle-missing-custom-architecturesCopy head branch name to clipboard

Conversation

@mattfaltyn

@mattfaltyn mattfaltyn commented Jul 15, 2026

Copy link
Copy Markdown

Linked issue

Fixes #6777

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • Performance improvement (includes benchmark results below)
  • Documentation update
  • New feature or public API (requires prior proposal or issue approval)
  • Refactor / internal cleanup (no user-visible change)
  • Build, CI, or tooling change

Motivation

Custom architecture modules that import successfully but omit the required
ARCHITECTURES list currently raise a raw AttributeError. This bypasses the
actionable ValueError already intended by the loader's validation branch.

What changed

  • Read ARCHITECTURES once with getattr() before validating it.
  • Reuse the validated list when registering custom architectures.
  • Add a focused regression test for a module missing ARCHITECTURES.

Testing

  • ./bazelw run format passed and made no changes.
  • ruff check max/python/max/pipelines/lib/registry.py max/tests/tests/pipelines/lib/test_registry.py
  • ruff format --check max/python/max/pipelines/lib/registry.py max/tests/tests/pipelines/lib/test_registry.py
  • Focused source-backed regression test: 1 passed.
  • Complete source-backed max/tests/tests/pipelines/lib package: 215 passed
    with 5 existing collection warnings.
  • ./bazelw test //max/tests/tests/pipelines/... //max/tests/integration/pipelines:tests
    built successfully: 5 tests passed and 32 macOS-incompatible tests were
    skipped.

Checklist

  • The linked issue above has been reviewed by a maintainer and is
    agreed-upon, or this is a trivial fix that does not need prior approval
  • PR is small and focused
  • I ran ./bazelw run format to format my changes
  • I added or updated tests to cover my changes
  • If AI tools assisted with this contribution, I have included an
    Assisted-by: trailer in my commit message or this PR description

Assisted-by: AI

@mattfaltyn
mattfaltyn force-pushed the agent/handle-missing-custom-architectures branch from 628c91d to 8cbb3e1 Compare July 15, 2026 08:35
@mattfaltyn

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

modular-cla-bot Bot added a commit to modular/cla that referenced this pull request Jul 15, 2026
@mattfaltyn
mattfaltyn force-pushed the agent/handle-missing-custom-architectures branch 2 times, most recently from 9a87782 to ba16074 Compare July 15, 2026 10:07
BEGIN_PUBLIC
[Pipelines] Handle missing custom ARCHITECTURES list

Return the intended actionable error when a custom architecture module omits its required ARCHITECTURES list, and cover the failure with a focused regression test.
END_PUBLIC

Assisted-by: AI
Signed-off-by: Matt Faltyn <faltyn.matthew@gmail.com>
@mattfaltyn
mattfaltyn force-pushed the agent/handle-missing-custom-architectures branch from ba16074 to a1a89be Compare July 15, 2026 10:20

Copy link
Copy Markdown
Author

The current Build/Test failure appears unrelated to this PR. The only failing target is //max/tests/tests/cascade:test_runtimes, which failed with an asyncio.CancelledError and timed out after 68.1 seconds; 1,987 tests passed. This PR only changes the custom architecture registry and its focused regression test, and does not touch Cascade. All relevant local tests are passing.

I attempted to rerun the failed job, but GitHub requires repository admin rights, so a maintainer will need to rerun the workflow.

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.

[BUG]: Missing ARCHITECTURES in custom module raises AttributeError instead of actionable ValueError

1 participant

Morty Proxy This is a proxified and sanitized view of the page, visit original site.