Skip to content

Conversation

@gerrod3
Copy link
Contributor

@gerrod3 gerrod3 commented Feb 5, 2025

I forgot to include this in my 3.70 compat PR (woops). Going to need to backport this to 3.13

content_artifact = content.contentartifact_set.first()
artifact = find_artifact()
origin = settings.CONTENT_ORIGIN.strip("/")
origin = settings.CONTENT_ORIGIN.strip("/") if settings.CONTENT_ORIGIN else ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If settings.CONTENT_ORIGIN was None, wouldn't this result in a URL that starts with /?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better to do something more along the lines of:

components = [prefix, base_path, content.filename]
if settings.CONTENT_ORIGIN:
    components.insert(0, settings.CONTENT_ORIGIN.strip("/"))
...

Which is the same way domain is handled more or less

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or skip the .insert() and instead just build the list component by component.

components = []
if settings.CONTENT_ORIGIN:
    components.append(settings.CONTENT_ORIGIN.strip("/"))
components.append(prefix)
if domain:
    components.append(domain.name)
components.append(base_path)
components.append(content.filename)

Which is probably simplest

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I updated it to try to match what I did in pypi/views, basically try the CONTENT_ORIGIN else assume same fqdn and use PYPI_API_HOSTNAME, else set to empty string and hope the client interacting with the pypi API can handle relative urls.

@gerrod3 gerrod3 merged commit 58653af into pulp:main Feb 11, 2025
12 checks passed
@gerrod3 gerrod3 deleted the 370-co-none branch February 11, 2025 14:51
@patchback
Copy link

patchback bot commented Feb 11, 2025

Backport to 3.13: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.13/58653afd4c8d961dfd179e3ddc5be908490449bb/pr-793

Backported as #796

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Feb 11, 2025
Ensure compatibility with CONTENT_ORIGIN=None

(cherry picked from commit 58653af)
gerrod3 added a commit that referenced this pull request Feb 11, 2025
…8d961dfd179e3ddc5be908490449bb/pr-793

[PR #793/58653afd backport][3.13] Ensure compatibility with CONTENT_ORIGIN=None
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.

2 participants