Skip to content

Migrate Python tooling to uv and ruff#1265

Merged
jhuleatt merged 1 commit intomainfrom
uv-ruff-migration-14229080439892494467
Apr 10, 2026
Merged

Migrate Python tooling to uv and ruff#1265
jhuleatt merged 1 commit intomainfrom
uv-ruff-migration-14229080439892494467

Conversation

@jhuleatt
Copy link
Copy Markdown
Collaborator

Migrates formatting for Python from yapf to ruff and uses uv pip install in github actions test steps to speed up tests, while keeping pip-compatibility for all samples.


PR created automatically by Jules for task 14229080439892494467 started by @jhuleatt

Changes included:
* Replace yapf with ruff in Python/requirements.txt and Python/pyproject.toml.
* Rewrite python formatter Python/pyfmt.py to use `ruff format -`.
* Format all Python code under Python/.
* Update CI at .github/workflows/test_python.yml to install uv and use `uv pip install --system` for speed.

Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request migrates the project's Python formatting from yapf to ruff, resulting in extensive stylistic updates across the codebase. Review feedback identifies a logic error in timezone handling for scheduled events and recommends replacing a bare except: block with a more specific exception handler. Additionally, the ruff command in the formatting script should explicitly reference the configuration file, and several network requests across the functions require timeouts to ensure reliability and prevent potential execution hangs.

Comment on lines +58 to +60
result = subprocess.run(
["ruff", "format", "-"], input=src.encode("utf-8"), capture_output=True, check=True
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The ruff command is called without specifying the configuration file. Since pyproject_toml is already defined in this script (and currently unused), it should be passed to ruff to ensure the formatting follows the project's rules. Additionally, consider using stdout=subprocess.PIPE instead of capture_output=True if you want to allow stderr (like warnings or errors) to be visible in the console instead of being swallowed.

Suggested change
result = subprocess.run(
["ruff", "format", "-"], input=src.encode("utf-8"), capture_output=True, check=True
)
result = subprocess.run(
["ruff", "format", "--config", pyproject_toml, "-"],
input=src.encode("utf-8"),
capture_output=True,
check=True,
)

@@ -85,8 +91,9 @@ def backupapod(req: tasks_fn.CallableRequest) -> str:
try:
pic_blob.upload_from_string(pic_resp.content, content_type=pic_type)
except:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Avoid using bare except: as it catches all exceptions, including SystemExit and KeyboardInterrupt, which can make debugging difficult and interfere with process termination. Use except Exception: instead to catch only standard exceptions.

Suggested change
except:
except Exception:

Comment on lines 112 to +113
"dateTime": (datetime.now() + timedelta(days=3)).isoformat(),
"timeZone": "America/Los_Angeles"
"timeZone": "America/Los_Angeles",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

In Cloud Functions, datetime.now() typically returns the time in UTC. Labeling this time with the America/Los_Angeles timezone in the Google Calendar API will result in the event being scheduled at the wrong time (shifted by the UTC offset). It is safer and more correct to use UTC as the timezone if you are using the system's current time.

Suggested change
"dateTime": (datetime.now() + timedelta(days=3)).isoformat(),
"timeZone": "America/Los_Angeles"
"timeZone": "America/Los_Angeles",
"dateTime": (datetime.now() + timedelta(days=3)).isoformat(),
"timeZone": "UTC",

Comment on lines 116 to +117
"dateTime": (datetime.now() + timedelta(days=3, hours=1)).isoformat(),
"timeZone": "America/Los_Angeles"
"timeZone": "America/Los_Angeles",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Similar to the start time, the end time should also use UTC to avoid a logic error caused by the mismatch between the UTC timestamp from datetime.now() and the hardcoded LA timezone.

Suggested change
"dateTime": (datetime.now() + timedelta(days=3, hours=1)).isoformat(),
"timeZone": "America/Los_Angeles"
"timeZone": "America/Los_Angeles",
"dateTime": (datetime.now() + timedelta(days=3, hours=1)).isoformat(),
"timeZone": "UTC",

Comment on lines 43 to 44
return requests.post(
url=webhook_url,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The requests.post call is missing a timeout. It's a best practice to always specify a timeout to prevent the function from hanging indefinitely if the external service is unresponsive.

    return requests.post(
        url=webhook_url,
        timeout=10,

Comment on lines +45 to +46
current_template = requests.get(
remote_config_api,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Missing timeout in requests.get. Adding a timeout ensures the function doesn't wait forever if the Remote Config API is slow or unreachable.

Suggested change
current_template = requests.get(
remote_config_api,
current_template = requests.get(
remote_config_api,
timeout=10,

Comment on lines +60 to +62
api_resp = requests.get(
url="https://api.nasa.gov/planetary/apod", params={"date": date, "api_key": NASA_API_KEY}
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Missing timeout in requests.get. It is recommended to set a timeout for all network requests in Cloud Functions to avoid unnecessary execution time and costs.

    api_resp = requests.get(
        url="https://api.nasa.gov/planetary/apod",
        params={"date": date, "api_key": NASA_API_KEY},
        timeout=10,
    )

Comment on lines +30 to +31
return requests.post(
SLACK_WEBHOOK_URL.value,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The requests.post call should include a timeout to prevent the function from being blocked indefinitely by a slow or non-responsive Slack webhook.

    return requests.post(
        SLACK_WEBHOOK_URL.value,
        timeout=10,

@jhuleatt jhuleatt merged commit d18b8ee into main Apr 10, 2026
7 checks passed
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.

1 participant