Skip to content

feat: Create sample for routeoptimization #12118

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bshi
Copy link

@bshi bshi commented Aug 7, 2024

Description

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

@bshi bshi requested review from a team as code owners August 7, 2024 21:53
Copy link

snippet-bot bot commented Aug 7, 2024

Here is the summary of changes.

You are about to add 1 region tag.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Aug 7, 2024
@bshi
Copy link
Author

bshi commented Aug 7, 2024

Tests blocked by API enablement (or enablement propagation)

@davidcavazos
Copy link
Contributor

Enabled the api in the testing project, re-running tests.

@davidcavazos davidcavazos added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 14, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 14, 2024
@davidcavazos
Copy link
Contributor

davidcavazos commented Aug 14, 2024

  File "/workspace/routeoptimization/snippets/optimize_tours_test.py", line 26, in test_call_sync_api
    for val in (got.routes, got.visits, got.metrics):
                            ^^^^^^^^^^
  File "/workspace/routeoptimization/snippets/.nox/py-3-12/lib/python3.12/site-packages/proto/message.py", line 906, in __getattr__
    raise AttributeError(
AttributeError: Unknown field for OptimizeToursResponse: visits

Looks like that field doesn't exist?

@bshi bshi requested a review from a team as a code owner January 8, 2025 18:41
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will provide feedback shortly. In the meantime, here's a summary for you and other reviewers to quickly get up to speed:

This pull request, authored by bshi, aims to add a new sample for the Route Optimization API. The changes include:

  • Added a new sample directory: routeoptimization/snippets/ containing code for optimizing tours using the Route Optimization API.
  • Updated CODEOWNERS: Line 120 was added to assign ownership of the new sample directory to @GoogleCloudPlatform/geo-routeoptimization, @GoogleCloudPlatform/python-samples-reviewers, and @GoogleCloudPlatform/cloud-samples-reviewers.
  • Updated blunderbuss.yml: Lines 330-332 were added to automatically assign reviewers for pull requests related to the Route Optimization API to @GoogleCloudPlatform/geo-routeoptimization.
  • Added a README.rst file: This file provides instructions on setting up the environment and running the sample code (lines 1-20).
  • Added noxfile_config.py: This file configures the testing environment (lines 1-42).
  • Added optimize_tours.py: This file contains the core logic for optimizing tours (lines 1-59).
  • Added optimize_tours_test.py: This file contains unit tests for the sample code (lines 1-28).
  • Added requirements-test.txt: Specifies testing dependencies (lines 1-2).
  • Added requirements.txt: Specifies runtime dependencies (line 1).

The author has indicated that the Route Optimization API needs to be enabled in testing projects, and the pull request includes a checklist to ensure all necessary steps are completed before merging. I'll be checking the test results and code quality during my review.


Here's a little haiku to brighten your day:

Routes optimize now,
Code flows smoothly, clear and bright,
Happy testing all!

@bshi
Copy link
Author

bshi commented Jan 8, 2025

@davidcavazos I apologize for the delay. Addressed your comments.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

This pull request adds a sample for the Route Optimization API. The sample code looks good, and the tests seem well-structured. However, there are a few minor improvements and a couple of missing checks that need to be addressed before merging.

@bshi bshi force-pushed the main branch 4 times, most recently from 195c19e to e3ada70 Compare January 8, 2025 19:12
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will provide my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed:

This pull request, authored by bshi, aims to add a new sample for the Route Optimization API. The changes include:

  • New sample directory: A new directory routeoptimization/snippets has been created to house the sample code.
  • Updated CODEOWNERS file (line 92): The CODEOWNERS file has been updated to assign ownership of the new sample directory to the @GoogleCloudPlatform/geo-routeoptimization, @GoogleCloudPlatform/python-samples-reviewers, and @GoogleCloudPlatform/cloud-samples-reviewers teams.
  • Updated blunderbuss.yml file (lines 261-265): The blunderbuss.yml file has been updated to correctly assign pull requests related to the Route Optimization API to the appropriate team.
  • New README.rst file (routeoptimization/snippets/README.rst): A new README file provides instructions for setting up and running the sample locally. (lines 1-20)
  • New Python sample file (routeoptimization/snippets/optimize_tours.py): This file contains the core logic for using the Route Optimization API. (lines 1-59)
  • New test file (routeoptimization/snippets/optimize_tours_test.py): This file includes tests to verify the functionality of the sample code. (lines 1-29)
  • New requirements files: requirements.txt and requirements-test.txt specify the necessary dependencies for the sample and its tests. (lines 1-2 and 1-1 respectively)
  • New noxfile_config.py: This file contains configuration for the nox testing framework. (lines 1-42)

The author has indicated that the Route Optimization API needs to be enabled in testing projects, and that tests pass using nox -s py-3.9 and nox -s lint.

I'll be checking the functionality and code quality shortly. Let me know if you have any questions in the meantime.


And now, a little haiku to brighten your day:

Routes optimize well,
Code flows smoothly, clear and bright,
Review's a swift breeze.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

The sample code and tests look good. A few minor suggestions for improvement are included below.

from google.maps import routeoptimization_v1


def call_optimize_tours(project_id: str) -> routeoptimization_v1.OptimizeToursResponse:
Copy link

Choose a reason for hiding this comment

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

low

It's a good practice to include a docstring briefly explaining what the function does.

Suggested change
def call_optimize_tours(project_id: str) -> routeoptimization_v1.OptimizeToursResponse:
def call_optimize_tours(project_id: str) -> routeoptimization_v1.OptimizeToursResponse:
"""Calls the optimize tours method."""


response = client.optimize_tours(
request=routeoptimization_v1.OptimizeToursRequest(
parent="projects/" + project_id,
Copy link

Choose a reason for hiding this comment

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

low

Use f-strings for better readability.

Suggested change
parent="projects/" + project_id,
parent=f"projects/{project_id}",

import optimize_tours


@Retry
Copy link

Choose a reason for hiding this comment

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

low

It is a good practice to include a docstring briefly explaining what the test does.

Suggested change
@Retry
@Retry
def test_call_sync_api() -> None:
"""Tests optimize tours method."""



def call_optimize_tours(project_id: str) -> routeoptimization_v1.OptimizeToursResponse:
# Use Default Application Credentials for the environment.
Copy link

Choose a reason for hiding this comment

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

low

It's a good practice to explicitly set the retry argument. Refer to this documentation for more details: https://googleapis.dev/python/google-api-core/latest/retry.html

Suggested change
# Use Default Application Credentials for the environment.
client = routeoptimization_v1.RouteOptimizationClient(client_options={'api_endpoint': 'us-central1-routeoptimization.googleapis.com:443'})



@Retry
def test_call_sync_api() -> None:
Copy link

Choose a reason for hiding this comment

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

low

It's a good practice to explicitly set the retry argument. Refer to this documentation for more details: https://googleapis.dev/python/google-api-core/latest/retry.html

Suggested change
def test_call_sync_api() -> None:
@Retry(deadline=600)
def test_call_sync_api() -> None:

@glasnt glasnt added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2025
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2025
@glasnt
Copy link
Contributor

glasnt commented Mar 5, 2025

Since this PR was created, we're introduced Python 3.13 testing. Please rebase this PR to ensure this check passes (if this API doesn't yet support Python 3.13, please update the noxfile to ignore this version)

@glasnt glasnt added the waiting-response Waiting for the author's response. label Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
samples Issues that are directly related to samples. waiting-response Waiting for the author's response.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants