Conversation
aproxy_address was unconditionally accessing runner_proxy_config.proxy_address, which fails when runner_proxy_config is None (aproxy not configured).
…elper - Set use_aproxy based on whether openstack_proxy is configured - Refactor wait_for to use a single async helper instead of duplicated branches
cbartz
requested review from
florentianayuwono,
javierdelapuente,
weiiwang01,
yanksyoon and
yhaliaw
as code owners
# Conflicts: # github-runner-manager/pyproject.toml
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes two bugs: (1) improper usage of aproxy when no proxy configuration is set, and (2) a bug in the wait_for helper function that prevented it from working correctly with lambda functions that return awaitables. The version is bumped from 0.12.0 to 0.12.1 to reflect these bug fixes.
Changes:
- Fixed
wait_forhelper to check if the result is awaitable rather than checking if the function is a coroutine function, enabling proper support for lambdas - Modified test factory to set
use_aproxybased on whetheropenstack_proxyis not None - Added defensive None check for
runner_proxy_configbefore accessingproxy_addressin cloud init generation
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/integration/helpers/common.py | Fixed wait_for helper to properly handle lambdas by checking if the result is awaitable instead of checking if the function is a coroutine function |
| github-runner-manager/tests/integration/factories.py | Set use_aproxy conditionally based on whether openstack_proxy is not None to prevent using aproxy without proxy configuration |
| github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py | Added None check for runner_proxy_config before accessing proxy_address to prevent AttributeError |
| github-runner-manager/pyproject.toml | Bumped version from 0.12.0 to 0.12.1 for patch release |
florentianayuwono
approved these changes
Feb 24, 2026
javierdelapuente
approved these changes
Feb 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applicable spec:
Overview
Fix the usage of aproxy even if no proxy configuration is set at all.
Also fix a bug with
wait_forhelper which doesn't work for lambdas (just returns true all the time), basically making some of our integration test useless.Rationale
For local testing, it can be useful to not use a proxy at all. This is where I encountered the issue.
wait_for bugfix is mixed into this PR, due to long-running/flaky CI which makes splitting functionality in multiple PR's quite inefficient
Juju Events Changes
n/a
Module Changes
n/a
Library Changes
Checklist
urgent,trivial,complex).github-runner-manager/pyproject.toml.