Skip to content

test: include Maven index lookups in interval experiment - #1

Closed
kdelee wants to merge 2 commits into
larrymou9:lrios/pulp-2346-megafrom
kdelee:kdelee/pulp-2346-maven-ab-coverage
Closed

kdelee wants to merge 2 commits into
larrymou9:lrios/pulp-2346-megafrom
kdelee:kdelee/pulp-2346-maven-ab-coverage

Conversation

@kdelee

@kdelee kdelee commented Oct 27, 2026 •

Copy link
Copy Markdown

Stacked on pulp-service pulp#1643

This change keeps the PULP-1996 A/B experiment and documents the Maven custom index-page path.

MavenDistribution.content_handler() uses RepositoryVersion.content for both generated index-page queries. RepositoryVersion.content delegates to RepositoryVersion.get_content(), so patch 0081 already dispatches these Maven queries between variants A and B. No additional runtime Maven patch is required.

The change:

  • documents Maven's A/B eligibility and its intentional bypass paths;
  • adds a regression test proving callers using RepositoryVersion.content receive the A/B dispatch;
  • keeps the full Maven content_handler() integration test as follow-up work in pulp_maven.

The previously proposed hosted 0082 patch was removed. It targeted the pinned pulp-maven==0.36.0 source shape, while current pulp_maven/main already uses the EXISTS membership shape; carrying that patch would make a future package bump fragile.

Maven path-index cache hits, Bloom-filtered misses, and ordinary artifact requests that fall through to the pulpcore handler remain outside the custom-handler A/B calls.

This is a stacked PR whose base is Larry's pulp#1643 branch:
lrios/pulp-2346-mega.

Validation:

  • scripts/ruff-diff-check.sh origin/main passes.
  • Ruff format check passes.

@kdelee

kdelee commented Oct 27, 2026 •

Copy link
Copy Markdown
Author

One clarification for the fallback path:

When MavenDistribution.content_handler() calls indexed_response() and it returns None (for example, no path-index descriptor or unavailable cache view), the request continues through the custom Maven handler:

  1. For a non-trailing-slash directory, the generated-index redirect probe uses version.get_content() and therefore dispatches PULP-1996 A/B.
  2. For a trailing-slash directory, the inline generated-index lookup also uses version.get_content() and dispatches PULP-1996 A/B.
  3. If the Maven lookup finds no usable index page, content_handler() returns None and pulpcore handles the request. Patch 0080 then applies its indexed relative_path + repository-membership EXISTS lookup.

This is different from the cases where indexed_response() raises a cached HTTPNotFound or returns a response; those intentionally do not reach either path.

The current test in this PR proves that RepositoryVersion.content dispatches through the experiment, but does not yet invoke MavenDistribution.content_handler() itself. I suggest validating the full Maven path with a focused plugin test:

  • create a Maven repository version containing a MavenIndexPage;
  • force indexed_response() to return None;
  • exercise both path and path/ requests;
  • assert both variants produce the same redirect/inline response;

Also exercise a path with no generated index page and verify that the custom handler returns None, allowing the pulpcore fallback to serve the artifact. For production validation, correlate Maven dispatch logs with RDS Performance Insights Top SQL using the PULP-1996 marker text. This should be done with the image containing pulp#1643 plus patch 0082, because the installed pulp_maven package must receive that patch.

@kdelee

kdelee commented Oct 27, 2026

Copy link
Copy Markdown
Author

The concerns are correct. The pinned service dependency is pulp-maven==0.36.0 (pulp_service/requirements.txt). The 0082 patch was written for that old content__in=version.content shape; it is not valid against current pulp_maven/main, which already uses the EXISTS shape. I removed 0082 and its Dockerfile application in f67a482. The remaining change documents the existing behavior: patch 0081 already covers Maven because version.content delegates to get_content(). The generic property-dispatch test remains useful, but a true Maven content_handler test belongs in pulp_maven. Therefore I would not merge the original 0082 version; the revised stacked PR is safe to fold into pulp#1643, or we can close this stacked PR and add only the documentation/test-plan clarification to pulp#1643.

@kdelee

kdelee commented Oct 27, 2026

Copy link
Copy Markdown
Author

I added the deployed-version fallback tests in a separate pulp_maven PR based on the 0.36 maintenance branch (the branch used for pulp-maven==0.36.0): pulp/pulp_maven#559. It forces indexed_response() to return None and covers: missing non-trailing index page, missing trailing-slash index page, and dotted artifact fallback. The tests verify Maven returns None to hand off to pulpcore and that directory lookups call version.content/get_content(); pulpcore #8270 tests cover the actual A/B SQL variants. I ran the focused tests in pr-1643-local with pulpcore at 2dcf2444e: 3 passed.

@kdelee kdelee closed this Oct 27, 2026
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.

1 participant