-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: add endpoint for getting collection metadata #424
Conversation
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.
Would it be possible to make this part of the CollectionViewSet in api.py instead? We try to use Django REST Framework where possible. I recently added http://127.0.0.1:8000/swagger-ui/ which automatically documents Django REST Framework APIs. In Pelican Backend's views.py, we made all methods use Django REST Framework, but dataset_filter_items
and dataset_distinct_values
were too unusual to fit into that framework. Not sure if this one is too unusual or not.
Noting that the 3 current endpoints could be changed to REST framework: #360 |
What is usual? The queries and the results of the queries we are performing here are very specific, e.g. there is not a model with the resulting fields, etc. |
I think this one is not too unusual. If you look at https://github.com/open-contracting/pelican-frontend/blob/main/backend/api/views.py some responses are just The two that are unusual have more request parameters (in the path, query string and/or request body). In principle, they can probably be aligned, I suppose. |
…ing/kingfisher-process into 421-data-registry-endpoint
… into 421-data-registry-endpoint
for more information, see https://pre-commit.ci
…ing/kingfisher-process into 421-data-registry-endpoint
I see precommit makes some fixes. Make sure you run |
Since you're converting the old endpoints, we'll need a PR in the data-registry as well. Another option is to keep |
We will need that PR to start using the new endpoint, so I will do both things in the same one |
@jpmckinney any idea why the open api test if failing with |
Cool - you can implement the new API in the registry, and update the others once this PR is reviewed.
I'll take a look. |
Fixed (had to switch to drf-spectacular, like in message) |
20a827b
to
801564e
Compare
- Use serializer instead of manual dicts - Use correct viewsets - Remove @csrf_exempt
…ke URLs more readable
801564e
to
1a00313
Compare
…nd until fa0ad29. The help text had not been updated to reflect this change.
…e API and Kingfisher Collect extension field docs. Align errors in create API and checker command.
…er can take the source's compiledRelease object, which might not have deleted null fields - Calculate the ocid_prefix in SQL - Extract dictfetchone() method I compared runtime for the old and new queries, and the new (fewer) queries are not noticeably slower (if at all).
…update_fields. Restore [] instead of .get() for required fields.
Thanks, I will prepare the PRs for Collect and Pelican, test them locally, and then merge and deploy the 3 projects together |
You mean the data registry? I don't think Pelican needs changes to be made in sync. |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
process/views.py
Outdated
SELECT DATA->>'publicationPolicy' AS publication_policy, | ||
DATA->>'license' AS license | ||
FROM package_data | ||
LEFT JOIN record r ON package_data.id = r.package_data_id | ||
AND r.collection_id = %(collection_id)s | ||
LEFT JOIN release r2 ON package_data.id = r2.package_data_id | ||
AND r2.collection_id = %(collection_id)s | ||
LIMIT 1 |
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.
@yolile If the collection has no matching record or release rows, this gets the first row in the package_data table (instead of no rows).
closes #421