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

Add package filename filter #3216

Merged
merged 1 commit into from
Aug 8, 2023
Merged

Conversation

daviddavis
Copy link
Contributor

fixes #3215

@@ -31,6 +31,7 @@ class Meta:
"arch": ["exact", "in", "ne", "contains", "startswith"],
"pkgId": ["exact", "in"],
"checksum_type": ["exact", "in", "ne"],
"location_href": ["exact", "contains", "endswith"],
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM functionality-wise, but I'm currently looking into whether we should ditch (or deprecate anyway) the location_href field in favor of something like "filename".

The reason being that location_href should never have existed in the current form to begin with, it stores the full path (think syncing), which doesn't really make much sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I knew that location_href stored the full path. I think that storing just the filename makes sense.

I'm pushing a change now that adds a "filename" filter. It looks like relative_path on the content artifact has just the filename (no path) so I think it should work and it won't depend on location_href.

@daviddavis daviddavis force-pushed the filter-by-location-href branch 2 times, most recently from 45e93b7 to 5bae3e9 Compare July 29, 2023 11:01
@daviddavis daviddavis changed the title Add location_href filters Add package filename filter Jul 31, 2023
Copy link
Contributor

@dralley dralley left a comment

Choose a reason for hiding this comment

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

At least at the moment I don't think this will work as-is because the relative path value isn't == filename, currently. But we should make it that way, IMO.

Or maybe I'm thinking of location_href and the relative path is just the filename. I need to check again...

@daviddavis
Copy link
Contributor Author

Did you see the link I posted? It looks like in the sync code at least, relative_path is set to os.path.basename(package.location_href).

filename = os.path.basename(package.location_href)
da = DeclarativeArtifact(
artifact=artifact,
url=url,
relative_path=filename,
remote=self.remote,
deferred_download=self.deferred_download,
)

@dralley dralley merged commit f30a3e8 into pulp:main Aug 8, 2023
14 checks passed
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.

Allow users to filter by location_href
2 participants