Basic interfaces for the deployers and the scaffolding of the factory.#7
Basic interfaces for the deployers and the scaffolding of the factory.#7richackard wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: richackard The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @richackard! |
k8s-ci-robot
added
size/L
| """ | ||
| # Imported lazily to keep this module decoupled from the concrete | ||
| # implementations, which may not be present yet during migration. | ||
| from deployers.gcp.gcp_deployer import GCPDeployer |
There was a problem hiding this comment.
Do we plan on migrating the GCPDeployer? I assumed this might be a good time to drop it
There was a problem hiding this comment.
Oh if we want to just stick with tf I am totally fine with it.
There was a problem hiding this comment.
Pull request overview
Introduces a new deployers package with a common Deployer interface and a get_deployer() factory, plus initial unit tests and test dependencies to validate deployer selection/precedence behavior while concrete deployers are still being migrated.
Changes:
- Added
deployers.base.DeployerABC anddeployers.factory.get_deployer()selection logic. - Added pytest-based tests that stub out (future) concrete deployer modules via
sys.modules. - Added a
testoptional dependency group and pytest configuration; updateduv.lockaccordingly.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
deployers/base.py |
Defines the abstract deployer interface used by the factory and future implementations. |
deployers/factory.py |
Adds deployer-selection logic (env/config precedence, stack defaulting, resolver choice). |
deployers/__init__.py |
Marks deployers as a Python package. |
tests/test_factory.py |
Adds tests for factory behavior using module stubs/test doubles. |
tests/__init__.py |
Marks tests as a package (supports import patterns used in the suite). |
pyproject.toml |
Adds test extras + pytest configuration. |
uv.lock |
Locks new test dependencies (pytest-mock) and metadata for extras. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| infra_config: dict[str, Any], | ||
| global_project_id: str, | ||
| global_cluster_name: str, | ||
| global_location: str = None, | ||
| ) -> Deployer: |
| # Imported lazily to keep this module decoupled from the concrete | ||
| # implementations, which may not be present yet during migration. | ||
| from deployers.gcp.gcp_deployer import GCPDeployer | ||
| from deployers.gcp.variables import resolve_variables as resolve_gcp_vars | ||
| from deployers.kind.variables import resolve_variables as resolve_kind_vars | ||
| from deployers.tf.tf_deployer import TFDeployer | ||
|
|
||
| provider_resolvers = { | ||
| "gcp": resolve_gcp_vars, | ||
| "kind": resolve_kind_vars, | ||
| } |
| def install(module_path, **attrs): | ||
| module = types.ModuleType(module_path) | ||
| for name, value in attrs.items(): | ||
| setattr(module, name, value) | ||
| monkeypatch.setitem(sys.modules, module_path, module) | ||
|
|
There was a problem hiding this comment.
Please address this comment as well
|
/label tide/merge-method-squash |
k8s-ci-robot
added
the
tide/merge-method-squash
Note the dummies in the factory will be replaced with the real deployers in the coming PRs.
| "kind": resolve_kind_vars, | ||
| } | ||
|
|
||
| # Respect task-level deployer first, fallback to cloud_provider if it is a valid deployer, otherwise default to terraform |
There was a problem hiding this comment.
Remove references to terraform
| """ | ||
| Factory to instantiate the appropriate infrastructure deployer. | ||
|
|
||
| Enforces GCP_LOCATION as the standard environment variable for location. |
There was a problem hiding this comment.
Use CLOUD_LOCATION instead of GCP_LOCATION
|
|
||
| return TFDeployer(tf_dir=stack, variables=variables) | ||
|
|
||
| # Fallback to legacy GCPDeployer (kubetest2) |
There was a problem hiding this comment.
This comment is a bit confusing/misleading. It's newly introduced here, so shouldn't be called "legacy". Also, when users specify deployer_type as gcp, it's not a fallback, but the explicitly requested behavior.
|
@richackard would you also take a look at Copilot review comments and resolve them? |
Note the dummies in the factory will be replaced with the real deployers in the coming PRs.