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

feat: feeds operations API function #838

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

Conversation

davidgamez
Copy link
Member

@davidgamez davidgamez commented Nov 28, 2024

Summary:

This function introduces the Operations API. This API is intended to be used by MobilityData internal team. The endpoints are protected by Google Oauth2.

Expected behavior:

The endpoints for updating GTFS and GTFS-RT are implemented.

Testing tips:

Via retool

  • Go to https://mobilitydata.retool.com/
  • Select development environment in the bottom left corner of the application
  • Open Feeds Dashboard application
  • List feeds and re-authenticate if necessary
  • Select a feed and open the feed screen
  • Make changes to the feed and save
  • Revert feed update changes so integration tests pointing to DEV are not affected

Via curl:
Replace the data with the desired values and populate the access token:

curl --request PUT \
  --url https://api-dev.mobilitydatabase.org//v1/operations/feeds/gtfs \
  --header 'Content-Type: ' \
--header 'Authorization: Bearer <YOUR_ACCESS_TOKEN_HERE> 
  --data '{
  "id": "mdb-1",
  "data_type": "gtfs",
  "status": "active",
  "created_at": "2023-07-10T22:06:00Z",
  "external_ids": [
    {
      "external_id": "10",
      "source": "mdb"
    }
  ],
  "provider": "Los Angeles Department of Transportation (LADOT, DASH, Commuter Express)",
  "feed_name": "Bus",
  "note": "A note to clarify complex use cases for consumers.",
  "feed_contact_email": "[email protected]",
  "source_info": {
    "producer_url": "https://ladotbus.com/gtfs",
    "authentication_type": 1,
    "authentication_info_url": "https://apidevelopers.ladottransit.com",
    "api_key_parameter_name": "Ocp-Apim-Subscription-Key",
    "license_url": "https://www.ladottransit.com/dla.html"
  },
  "redirects": [
    {
      "target_id": "mdb-100",
      "comment": "Redirected because of a change of URL."
    }
  ]
}'

From our AI friend

This pull request includes a variety of changes to the API deployment workflows, the feeds API implementation, and the documentation. The most important changes include the addition of OAuth2 client ID handling, the generation and upload of Operations API code, and the inclusion of logging in the feeds API implementation.

Workflow Enhancements:

  • Added OPERATIONS_OAUTH2_CLIENT_ID_1PASSWORD to the environment variables in .github/workflows/api-deployer.yml and .github/workflows/api-dev.yml to handle OAuth2 client ID for the operations API. [1] [2]
  • Included steps to generate and upload Operations API code in .github/workflows/build-test.yml. [1] [2]

Feeds API Implementation:

  • Introduced a logger in api/src/feeds/impl/feeds_api_impl.py and added logging for user email restriction checks. [1] [2]
  • Updated methods in api/src/feeds/impl/feeds_api_impl.py to use the logger and refactored email restriction checks. [1] [2] [3] [4]

Documentation:

  • Added docs/OperationsAPI.yaml to document the Operations API, including endpoints for updating GTFS and GTFS-RT feeds.

Miscellaneous:

  • Updated api/src/middleware/request_context.py to correct the domain check in is_user_email_restricted function.
  • Modified the test for is_user_email_restricted in api/tests/unittest/middleware/test_request_context.py to reflect the corrected domain check.
  • Updated .flake8 and added .gcloudignore to exclude new directories and files from linting and Google Cloud uploads respectively. [1] [2]

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with ./scripts/api-tests.sh to make sure you didn't break anything
  • Add or update any needed documentation to the repo
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

rel.cascade = "all, delete-orphan"


event.listen(mapper, "mapper_configured", set_cascade)
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this be executed when the file is imported?Is this what you want?

Copy link
Member Author

@davidgamez davidgamez Dec 2, 2024

Choose a reason for hiding this comment

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

It will be executed when the SQLAlchemy triggers the mapper configurated events. This is not exactly when the file is imported, it's part of the SqlAchemy lifecycle. It might happen once per thread while Sqlalchemy is instantiating the classes.

Comment on lines 114 to 115
unrestricted_domains = ["@mobilitydata.org"]
unrestricted_domains = ["mobilitydata.org"]
return not email or not any(email.endswith(f"@{domain}") for domain in unrestricted_domains)
Copy link
Member Author

Choose a reason for hiding this comment

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

This fixes the unrestricted logic as it was adding two times the @ character

@davidgamez davidgamez marked this pull request as ready for review December 4, 2024 18:33
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.

2 participants