-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add license information to operations API #1506
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
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.
Pull request overview
This PR adds comprehensive license management capabilities to the operations API, enabling users to query license information and associate licenses with feeds. The changes introduce a new /v1/operations/licenses endpoint family and extend feed creation/update operations to support license metadata.
Key Changes:
- New Licenses API with endpoints for listing and retrieving license details including rules
- Extended GTFS and GTFS-RT feed models to support license_id, license_notes, and license_is_spdx fields
- Database performance optimizations via PostgreSQL trigram indexes for license search queries
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| liquibase/changes/feat_1432_*.{xml,sql} | Database migration files adding pg_trgm extension and concurrent indexes for license search optimization |
| liquibase/changelog.xml | Registers new license index migrations |
| functions-python/operations_api/src/feeds_operations/impl/licenses_api_impl.py | Core implementation of Licenses API with get_license and get_licenses endpoints |
| functions-python/operations_api/tests/feeds_operations/impl/test_licenses_api_impl.py | Comprehensive test suite for license API operations |
| functions-python/operations_api/src/main.py | Registers new LicenseApiRouter |
| functions-python/operations_api/src/feeds_operations/impl/models/update_request_gtfs*_feed_impl.py | Extends feed models to include license fields in ORM conversions |
| functions-python/operations_api/src/feeds_operations/impl/feeds_operations_impl.py | Adds error handling for dataset download failures |
| functions-python/operations_api/.openapi-generator/FILES | Tracks newly generated license-related API files |
| docs/OperationsAPI.yaml | Adds OpenAPI specifications for license endpoints and schemas |
| api/src/shared/db_models/feed_impl.py | Updates feed ORM conversion to lookup and validate license_id references |
| functions-python/operations_api/tests/conftest.py | Adds test fixtures for licenses and rules |
| functions-python/operations_api/tests/feeds_operations/impl/test_feeds_operations_impl_gtfs.py | Updates GTFS test to include license fields |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
| ) | ||
| try: | ||
| trigger_dataset_download( | ||
| created_feed, |
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.
Pycharm complains that created_feed could be None, and trigger_dataset_download does not support receiving a None.
We should guard against this.
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 added a guard; unfortunately, PyCharm still triggers the warning even though the condition guarantees that None is not present at runtime.
| result = await api.get_licenses(limit=10, offset=0, search_query=fragment) | ||
|
|
||
| assert isinstance(result, list) | ||
| assert any(item.id == existing.id for item in result) |
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.
A bit of a nitpick, but strictly speaking this is not testing that any license that should not be there is effectively not there (false positive)
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.
Added Thanks!
jcpitre
left a comment
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.
LGTM!
* add sentry to web-app * Sentry trace rate parsing safety Co-authored-by: Copilot <[email protected]> * co pilot pr comments * deployment settings * update variable script to allow optional variables * error grouping and error detail * gbfs validator: error grouping and detail * errorDetailDialog refactor * ui of error dialog * tooltip adjustment * mobile support styling * accessibility fixes * styling touchup * updated test data * lint * co-pilot pr comments * improved UI to help find error details * focus fix * strict null checks for auth header builder * allow empty username and password * styling adjustment --------- Co-authored-by: Copilot <[email protected]>
…#1501) * changed feed references logic
|
*Lighthouse ran on https://mobility-feeds-dev--pr-1506-zfkz4at5.web.app/ * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1506-zfkz4at5.web.app/feeds * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1506-zfkz4at5.web.app/feeds/gtfs/mdb-2126 * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1506-zfkz4at5.web.app/feeds/gtfs_rt/mdb-2585 * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1506-zfkz4at5.web.app/gbfs/gbfs-flamingo_porirua * (Desktop)
|
|
Preview Firebase Hosting URL: https://mobility-feeds-dev--pr-1506-zfkz4at5.web.app |
Summary:
Part of #1432
As a follow-up to #1482, this PR adds support for license information in the operations API. Main changes:
/v1/operations/licenses: returns all licenses without rules/v1/operations/licenses/{license_id}: return the information for a specific license, including the rules array/v1/operations/feeds/gtfs_rtand/v1/operations/feeds/gtfssupporting license_id, license_is_spdx and license_notes fields.From our AI friend
This pull request introduces a new Licenses API to the Mobility Database Operations service, adds models and endpoints for querying license information, and extends feed-related models and endpoints to better support license metadata. The changes cover API documentation, backend implementation, and model updates to enable listing, searching, and retrieving license details, including license rules and SPDX status.
Licenses API Implementation
LicensesApiImplclass that provides endpoints for listing licenses (get_licenses) and retrieving a specific license with its rules (get_license). This supports pagination, search, and SPDX filtering.licenses_api.py,license_base.py, etc.). [1] [2]API Documentation and OpenAPI Spec Updates
/v1/operations/licensesand/v1/operations/licenses/{id}endpoints to the OpenAPI spec, including request parameters for pagination, search, and SPDX filtering, and response schemas for license details and rules. [1] [2]LicenseBase,LicenseRule,LicenseWithRules, and updated feed schemas to include license-related fields (license_id,license_notes,license_is_spdx). [1] [2]Feed Model and ORM Updates
license_id,license_notes, andlicense_is_spdxfields, including proper lookup and error handling for missing licenses. [1] [2] [3] [4]Error Handling and Logging Improvements
Expected behavior:
License is returned in the get method and it's included in the create and update methods
Testing tips:
Local testing
Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.shto make sure you didn't break anything