-
Notifications
You must be signed in to change notification settings - Fork 26
Description
Checklist
- This refactor maintains backward compatibility with all user-facing APIs.
- For large-scale refactors, I’ve prepared a phased implementation plan.
Current Limitations
The current Go module structure and package naming do not clearly reflect their responsibilities or abstraction levels, leading to confusion and tight coupling:
servicepackage is misnamed — it acts as the core execution engine, not a generic service layer.controllerimplies MVC semantics that don’t fully align with Go's idiomatic API design; it serves as the HTTP/gRPC entrypoint layer.service/env_instance.gomixes interface definition (EnvInstance) with concrete implementation, making it hard to swap out backends or write clean unit tests.schedule_clientis ambiguously named and hides its actual purpose: managing sandboxed environments on Kubernetes.- Import paths and dependencies are tightly coupled, increasing the cost of future engine extensibility (e.g., adding Ray, Docker, or E2B backends).
These issues hinder code readability, testability, and long-term maintainability.
Proposed Refactor
Refactor package names and internal structure to improve clarity, enforce separation between interface and implementation, and support future pluggable engines:
-
Rename
service→engine- Update package name and directory:
pkg/service→pkg/engine - All types like
ServiceClient,NewService()→EngineClient,NewEngine() - Benefit: Clearer domain semantics — this package encapsulates execution logic, not general-purpose services.
- Update package name and directory:
-
Rename
controller→api- Update:
pkg/controller→pkg/api - Refactor HTTP handlers and routers accordingly (e.g.,
Controller.ServeHTTP→API.Serve) - Benefit: More accurately represents the role as the external-facing interface layer.
- Update:
-
Split
service/env_instance.gointo two files underengine:env_instance_interface.go:type EnvInstance interface { Start(ctx context.Context) error Stop(ctx context.Context) error Exec(ctx context.Context, cmd []string) ([]byte, error) // ... }
standard_engine_client.go:
ImplementsEnvInstancewith current logic.- Move interface to be part of
enginepackage; keep implementation concrete but decoupled. - Benefit: Enables mocking in tests and paves the way for alternative implementations (e.g.,
ray_engine_client,e2b_engine_client).
-
Rename
schedule_client→k8s_sandbox_client- File and package:
pkg/schedule_client→pkg/k8s_sandbox_client - Type:
ScheduleClient→K8SSandboxClient - Clearly signals Kubernetes + sandboxing context.
- Benefit: Prevents misinterpretation and improves discoverability.
- File and package:
-
Update all internal imports and dependency references
- Use IDE-assisted refactoring or
gopls rename/go mod editwhere applicable. - Ensure no circular dependencies are introduced.
- Validate with
go build ./...andgo test ./...post-refactor.
- Use IDE-assisted refactoring or
Benefits:
- Clearer architecture:
apidepends onengine, which depends on interfaces, not concrete clients. - Supports upcoming multi-engine strategy via interface-based design.
- More idiomatic Go structure with explicit abstractions.
- Easier testing and dependency injection.
Alternatives Considered
-
Introduce intermediate facade instead of renaming:
Could wrap old packages, but would add unnecessary indirection and delay necessary cleanup. -
Use versioned packages (e.g.,
v2) for backward compatibility:
Not needed — all affected packages are internal (pkg/...), with no external module exposure or tagged releases depending on them. -
Keep combined
env_instancefile with internal interface:
Violates Go best practices for large interfaces; splitting improves clarity and aligns with standard library patterns (e.g.,io.Reader,http.Handler).
Additional Context
This change supports the roadmap for pluggable sandbox engines (see #123).
Related discussion: Design Proposal: Pluggable Execution Engines
🔧 Implementation Notes (Go-specific):
- Use
goplsorgomodifytags+goimportsto safely update references. - Consider adding
//go:build ignoreor feature flags if rolling out in phases. - Update any documentation, examples, or
README.mdreferencing old package names.
📌 Next Steps:
- Open draft PR with proposed directory and package renames.
- Run linter and CI pipeline to catch import errors early.
- Add interface-based unit tests to validate abstraction quality.