-
Notifications
You must be signed in to change notification settings - Fork 23
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
Create public purl-validate UI #535
base: main
Are you sure you want to change the base?
Conversation
Reference: #522 Signed-off-by: John M. Horan <[email protected]>
Reference: #522 Signed-off-by: John M. Horan <[email protected]>
Reference: #522 Signed-off-by: John M. Horan <[email protected]>
Reference: #522 Signed-off-by: John M. Horan <[email protected]>
Reference: #522 Signed-off-by: John M. Horan <[email protected]>
@JonoYang I've run |
In the CI output:
It's complaining that the import statements in packagedb/models.py are not sorted. You can run |
@JonoYang Well, as I noted, I ran |
Rerunning |
Reference: #522 Signed-off-by: John M. Horan <[email protected]>
. . . and all checks have now passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnmhoran Thanks for the PR! I've made a pass on the Python code portion.
return serializer.data | ||
|
||
qs = qs.filter( | ||
models.Q(namespace=package_url.namespace) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move these if-else statements into their own variables for clarity.
if print_to_console: | ||
print(f"\nmodels.py PackageQuerySet search() qs.query = {qs.query}") | ||
print(f"\nlist(qs): {list(qs)}") | ||
for abc in list(qs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use a variable name that describes what's in qs
.
|
||
return qs | ||
|
||
def get_packageurl_from_string(self, query: str = None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this method being used anywhere?
@@ -81,6 +87,105 @@ def paginated(self, per_page=5000): | |||
page = paginator.page(page_number) | |||
yield from page.object_list | |||
|
|||
# Based on class PurlValidateResponseSerializer(Serializer). Added here because when added to serializers.py and imported raises a circular import error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that the circular import happens when you add this code to serializers.py and then try to import it from somewhere like views.py?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JonoYang Glad you asked about the custom serializer I created inside models.py
.
This means that when I initially defined that new serializer in packagedb/serializers.py
and tried to import it into models.py
, where I am using the local one now, I got the circular import error. Maybe I don't need a serializer here?
I modeled my approach on what we do in the api.py
PurlValidateViewSet()
class with the PurlValidateResponseSerializer
we import from packagedb.serializers
. Essentially I'm trying to mimic in my purl-validate UI the dictionary that our validate endpoint returns -- 4 fields, and I added one or two more for my UI validation messaging (preventing an exception from being raised and interrupting the process). packagedb/serializers.py
has numerous imports from packagedb/models
, so that's the source of the circular import issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JonoYang You can see the similar code structure I used and the validate endpoint used to handle an exception:
print(f"\nviews.py parse_purl() query = {query}") | ||
|
||
purl_error_message = "" | ||
purl_pkg_scheme_component = "AAA" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can leave these as empty strings
print(f"ValueError = {ValueError(msg)}") | ||
|
||
# ==> namespace component === | ||
namespace_01 = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using numbers in variable names
purl_error_message = "The input purl is valid." | ||
|
||
# ==> type component === | ||
purl_type = "MISSING" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like you can leave these as empty strings and then check if they exist using if purl_type
or if not purl_type
Reference: #522
This preserves the initial demo version. Next: begin to implement latest feedback.