Skip to content
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

As a plugin writer, I have to opt-out to having a "non-redirect" behavior in the content app #3459

Closed
bmbouter opened this issue Dec 13, 2022 · 6 comments · Fixed by #4468
Closed

Comments

@bmbouter
Copy link
Member

With the pulpcore 3.22 release, we planned to make the "redirect to include a trailing slash" behavior a plugin opt-in feature. We couldn't make it opt-out because pulp_container breaks with this and pre-existing GA releases of pulp_container declare forward compatibility through pulpcore 3.24.

This issue tracks the change: With 3.25, have the "redirect to include a trailing slash" be a plugin opt-out feature and have it enabled by default.

@bmbouter bmbouter added this to the 3.25.0 blockers milestone Dec 13, 2022
@bmbouter bmbouter changed the title As a plugin writer, I have to opt-in to having a "non-redirect" behavior in the content app As a plugin writer, I have to opt-out to having a "non-redirect" behavior in the content app Dec 13, 2022
@bmbouter
Copy link
Member Author

@pulp/core FYI

@daviddavis
Copy link
Contributor

Just adding some historical notes here.

The original redirect feature issue: #3173

The original PR to add the feature: #3434

The PR that reverted/removed the feature: #3465

@ggainey
Copy link
Contributor

ggainey commented May 2, 2023

Clarification: implementing this requires more than just re-pushing the PR - it requires adding machinery to let plugins opt in/out of the behavior

@lubosmj
Copy link
Member

lubosmj commented May 4, 2023

I do not understand the need to enable or disable the feature in the plugin's code. We decided to revert the commit because we had compatibility issues in pulp_container (#3434 (comment)) and pulp_python (#3456). The branch where the commit was merged had not been identified as a branch holding breaking changes.

Is it acceptable to merge the very same commit with, for instance, a hook method that will enable plugin writers to create an additional wrap-around on top of the non-redirect workflow, if needed? This is what we were trying to add to pulp_container to fix the issues: pulp/pulp_container#1179.

@lubosmj
Copy link
Member

lubosmj commented May 4, 2023

Is this classified as a duplicate of #3173 now?

@mdellweg mdellweg self-assigned this May 4, 2023
mdellweg pushed a commit to mdellweg/pulpcore that referenced this issue May 4, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

Changes the order in which pulpcore-content looks for published artifacts
The handler now looks for a published artifact before it tries to look for an index.html or listing
the directory.

fixes: pulp#3173
fixes: pulp#3459
mdellweg pushed a commit to mdellweg/pulpcore that referenced this issue May 5, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

Changes the order in which pulpcore-content looks for published artifacts
The handler now looks for a published artifact before it tries to look for an index.html or listing
the directory.

fixes: pulp#3173
fixes: pulp#3459
mdellweg pushed a commit to mdellweg/pulpcore that referenced this issue May 5, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

Changes the order in which pulpcore-content looks for published artifacts
The handler now looks for a published artifact before it tries to look for an index.html or listing
the directory.

fixes: pulp#3173
fixes: pulp#3459
mdellweg pushed a commit to mdellweg/pulpcore that referenced this issue May 5, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

Changes the order in which pulpcore-content looks for published artifacts
The handler now looks for a published artifact before it tries to look for an index.html or listing
the directory.

fixes: pulp#3173
fixes: pulp#3459
mdellweg pushed a commit to mdellweg/pulpcore that referenced this issue May 8, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

Changes the order in which pulpcore-content looks for published artifacts
The handler now looks for a published artifact before it tries to look for an index.html or listing
the directory.

fixes: pulp#3173
fixes: pulp#3459
mdellweg pushed a commit to mdellweg/pulpcore that referenced this issue May 9, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

Changes the order in which pulpcore-content looks for published artifacts
The handler now looks for a published artifact before it tries to look for an index.html or listing
the directory.

fixes: pulp#3173
fixes: pulp#3459

Co-authored-by: bmbouter <bmbouter@gmail.com>
@lubosmj
Copy link
Member

lubosmj commented Jun 27, 2023

This is the API change required for #3173.

dkliban added a commit to dkliban/pulpcore that referenced this issue Sep 22, 2023
dkliban added a commit to dkliban/pulpcore that referenced this issue Sep 22, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Sep 22, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Sep 22, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Sep 22, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Sep 27, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Sep 27, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Oct 26, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Oct 30, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Oct 30, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Oct 31, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Oct 31, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit to dkliban/pulpcore that referenced this issue Oct 31, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
dkliban added a commit that referenced this issue Oct 31, 2023
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: #3173
closes: #3459
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment