-
Notifications
You must be signed in to change notification settings - Fork 0
feat: AE-1018 Add support for new serverless runtime #26
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: pandyamarut <[email protected]>
9bc7a41 to
1eff33e
Compare
Resolved uv.lock conflicts by regenerating lock file with current dependencies.
80f3358 to
68df25d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for a new serverless runtime called "Load Balancer SLS" that provides dual-capability execution - both traditional remote execution and HTTP endpoint exposure. The implementation introduces a FastAPI-based server that can handle both programmatic calls via RemoteExecutor and HTTP requests to decorated class methods.
- Implements Load Balancer SLS server with dual execution capabilities (remote + HTTP endpoints)
- Adds @endpoint decorator for marking class methods as HTTP endpoints
- Creates class registry system for managing deployed classes and their endpoint mappings
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tetra-rp | Updates subproject commit reference |
| src/load_balancer_sls_handler.py | Main server implementation with FastAPI integration and dual execution support |
| src/endpoint.py | HTTP endpoint decorator for marking class methods as web endpoints |
| src/class_registry.py | Class management system for registration, instantiation, and method execution |
| pyproject.toml | Adds FastAPI, uvicorn, and aiohttp dependencies |
| Makefile | Adds build and test targets for Load Balancer SLS image |
| Dockerfile-load-balancer-sls | Docker configuration for the new serverless runtime |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| # Use RemoteExecutor for ALL executions to ensure dependency installation | ||
| log.debug( | ||
| f"Remote execution: {getattr(input_data, 'class_name', input_data.function_name)}" |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line will raise an AttributeError if input_data.function_name doesn't exist when class_name is also missing. The getattr call should have a default value for the fallback case.
| f"Remote execution: {getattr(input_data, 'class_name', input_data.function_name)}" | |
| f"Remote execution: {getattr(input_data, 'class_name', None) or getattr(input_data, 'function_name', None) or '<unknown>'}" |
| # Also update by class name for default instances | ||
| for key, metadata in self.instance_metadata.items(): | ||
| if metadata.get("class_name") == instance_key: | ||
| metadata["method_calls"] += 1 | ||
| metadata["last_used"] = datetime.now().isoformat() | ||
|
|
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic in lines 298-301 is incorrect. It's comparing metadata.get('class_name') with instance_key, but instance_key is an instance identifier, not a class name. This will never match and the metadata updates will be duplicated or missed.
| # Also update by class name for default instances | |
| for key, metadata in self.instance_metadata.items(): | |
| if metadata.get("class_name") == instance_key: | |
| metadata["method_calls"] += 1 | |
| metadata["last_used"] = datetime.now().isoformat() |
| from typing import List, Optional | ||
|
|
||
|
|
||
| def endpoint(methods: List[str] = ["POST"], route: Optional[str] = None): |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a mutable list as a default argument is an anti-pattern. The list should be None with a fallback inside the function: methods: Optional[List[str]] = None and then methods = methods or ['POST'] in the function body.
|
|
||
| # For HTTP calls, pass request data as keyword arguments | ||
| if isinstance(request_data, dict): | ||
| result = method(**request_data) |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line assumes request_data is always a valid dictionary for keyword expansion. If request_data contains keys that don't match the method's parameters, this will raise a TypeError. Consider adding parameter validation or using inspect.signature() to filter valid parameters.
| result = method(**request_data) | |
| sig = inspect.signature(method) | |
| params = sig.parameters | |
| accepts_kwargs = any( | |
| p.kind == inspect.Parameter.VAR_KEYWORD | |
| for p in params.values() | |
| ) | |
| if accepts_kwargs: | |
| filtered_data = request_data | |
| else: | |
| filtered_data = { | |
| k: v | |
| for k, v in request_data.items() | |
| if k in params | |
| } | |
| result = method(**filtered_data) |
No description provided.