-
Notifications
You must be signed in to change notification settings - Fork 6
fix: resolve .env from CWD and stop _do_deploy env mutation #236
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?
Changes from all commits
6bb6348
5666355
03733ba
02b498a
53a4806
788808e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -259,20 +259,19 @@ def validate_python_version(cls, v: Optional[str]) -> Optional[str]: | |||||||||||||||
|
|
||||||||||||||||
| @property | ||||||||||||||||
| def config_hash(self) -> str: | ||||||||||||||||
| """Get config hash excluding env and runtime-assigned fields. | ||||||||||||||||
| """Get config hash excluding runtime-assigned fields. | ||||||||||||||||
|
|
||||||||||||||||
| Prevents false drift from: | ||||||||||||||||
| - Dynamic env vars computed at runtime | ||||||||||||||||
| - Runtime-assigned fields (template, templateId, aiKey, userId, etc.) | ||||||||||||||||
|
|
||||||||||||||||
| Only hashes user-specified configuration, not server-assigned state. | ||||||||||||||||
| Hashes user-specified configuration including env vars. | ||||||||||||||||
| """ | ||||||||||||||||
| import hashlib | ||||||||||||||||
| import json | ||||||||||||||||
|
|
||||||||||||||||
| resource_type = self.__class__.__name__ | ||||||||||||||||
|
|
||||||||||||||||
| # Exclude runtime fields, env, and id from hash | ||||||||||||||||
| # Exclude runtime fields and id from hash | ||||||||||||||||
| exclude_fields = ( | ||||||||||||||||
| self.__class__.RUNTIME_FIELDS | self.__class__.EXCLUDED_HASH_FIELDS | ||||||||||||||||
| ) | ||||||||||||||||
|
|
@@ -534,12 +533,24 @@ def _payload_exclude(self) -> Set[str]: | |||||||||||||||
|
|
||||||||||||||||
| @staticmethod | ||||||||||||||||
| def _build_template_update_payload( | ||||||||||||||||
| template: PodTemplate, template_id: str | ||||||||||||||||
| template: PodTemplate, | ||||||||||||||||
| template_id: str, | ||||||||||||||||
| *, | ||||||||||||||||
| skip_env: bool = False, | ||||||||||||||||
| ) -> Dict[str, Any]: | ||||||||||||||||
| """Build saveTemplate payload from template model. | ||||||||||||||||
|
|
||||||||||||||||
| Keep this to fields supported by saveTemplate to avoid passing endpoint-only | ||||||||||||||||
| fields to the template mutation. | ||||||||||||||||
|
|
||||||||||||||||
| Args: | ||||||||||||||||
| template: Template model with desired configuration. | ||||||||||||||||
| template_id: ID of the template to update. | ||||||||||||||||
| skip_env: When True, omit ``env`` from the payload so | ||||||||||||||||
| saveTemplate preserves the existing template env vars. | ||||||||||||||||
| This prevents removing platform-injected vars (e.g. | ||||||||||||||||
| PORT, PORT_HEALTH on LB endpoints) when the user's | ||||||||||||||||
| env hasn't actually changed. | ||||||||||||||||
| """ | ||||||||||||||||
| template_data = template.model_dump(exclude_none=True, mode="json") | ||||||||||||||||
| allowed_fields = { | ||||||||||||||||
|
|
@@ -550,6 +561,8 @@ def _build_template_update_payload( | |||||||||||||||
| "env", | ||||||||||||||||
| "readme", | ||||||||||||||||
| } | ||||||||||||||||
| if skip_env: | ||||||||||||||||
| allowed_fields.discard("env") | ||||||||||||||||
| payload = { | ||||||||||||||||
| key: value for key, value in template_data.items() if key in allowed_fields | ||||||||||||||||
| } | ||||||||||||||||
|
|
@@ -643,6 +656,56 @@ def _get_module_path(self) -> Optional[str]: | |||||||||||||||
| except Exception: | ||||||||||||||||
| return None | ||||||||||||||||
|
|
||||||||||||||||
| def _inject_template_env(self, key: str, value: str) -> None: | ||||||||||||||||
| """Append a KeyValuePair to self.template.env if the key isn't already present. | ||||||||||||||||
|
|
||||||||||||||||
| This injects runtime env vars directly into the template without | ||||||||||||||||
| mutating self.env, which would cause false config drift on subsequent | ||||||||||||||||
| deploys. | ||||||||||||||||
| """ | ||||||||||||||||
| if self.template is None: | ||||||||||||||||
| return | ||||||||||||||||
| if self.template.env is None: | ||||||||||||||||
| self.template.env = [] | ||||||||||||||||
| existing_keys = {kv.key for kv in self.template.env} | ||||||||||||||||
| if key not in existing_keys: | ||||||||||||||||
| self.template.env.append(KeyValuePair(key=key, value=value)) | ||||||||||||||||
|
|
||||||||||||||||
| def _inject_runtime_template_vars(self) -> None: | ||||||||||||||||
| """Inject runtime env vars into template.env without mutating self.env. | ||||||||||||||||
|
|
||||||||||||||||
| For QB endpoints making remote calls: injects RUNPOD_API_KEY. | ||||||||||||||||
| For LB endpoints: injects FLASH_MODULE_PATH. | ||||||||||||||||
|
|
||||||||||||||||
| Called by both _do_deploy (initial) and update (env changes) so | ||||||||||||||||
| runtime vars survive template updates. | ||||||||||||||||
| """ | ||||||||||||||||
| env_dict = self.env or {} | ||||||||||||||||
|
|
||||||||||||||||
| if self.type == ServerlessType.QB: | ||||||||||||||||
| if self._check_makes_remote_calls(): | ||||||||||||||||
| if "RUNPOD_API_KEY" not in env_dict: | ||||||||||||||||
| from runpod_flash.core.credentials import get_api_key | ||||||||||||||||
|
|
||||||||||||||||
| api_key = get_api_key() | ||||||||||||||||
| if api_key: | ||||||||||||||||
| self._inject_template_env("RUNPOD_API_KEY", api_key) | ||||||||||||||||
| log.debug( | ||||||||||||||||
| f"{self.name}: Injected RUNPOD_API_KEY for remote calls " | ||||||||||||||||
| f"(makes_remote_calls=True)" | ||||||||||||||||
| ) | ||||||||||||||||
| else: | ||||||||||||||||
| log.warning( | ||||||||||||||||
| f"{self.name}: makes_remote_calls=True but RUNPOD_API_KEY not set. " | ||||||||||||||||
| f"Remote calls to other endpoints will fail." | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| elif self.type == ServerlessType.LB: | ||||||||||||||||
| module_path = self._get_module_path() | ||||||||||||||||
| if module_path and "FLASH_MODULE_PATH" not in env_dict: | ||||||||||||||||
| self._inject_template_env("FLASH_MODULE_PATH", module_path) | ||||||||||||||||
| log.debug(f"{self.name}: Injected FLASH_MODULE_PATH={module_path}") | ||||||||||||||||
|
|
||||||||||||||||
| async def _do_deploy(self) -> "DeployableResource": | ||||||||||||||||
| """ | ||||||||||||||||
| Deploys the serverless resource using the provided configuration. | ||||||||||||||||
|
|
@@ -658,43 +721,7 @@ async def _do_deploy(self) -> "DeployableResource": | |||||||||||||||
| log.debug(f"{self} exists") | ||||||||||||||||
| return self | ||||||||||||||||
|
|
||||||||||||||||
| # Inject API key for queue-based endpoints that make remote calls | ||||||||||||||||
| if self.type == ServerlessType.QB: | ||||||||||||||||
| env_dict = self.env or {} | ||||||||||||||||
|
|
||||||||||||||||
| # Check if this resource makes remote calls (from build manifest) | ||||||||||||||||
| makes_remote_calls = self._check_makes_remote_calls() | ||||||||||||||||
|
|
||||||||||||||||
| if makes_remote_calls: | ||||||||||||||||
| # Inject RUNPOD_API_KEY if not already set | ||||||||||||||||
| if "RUNPOD_API_KEY" not in env_dict: | ||||||||||||||||
| from runpod_flash.core.credentials import get_api_key | ||||||||||||||||
|
|
||||||||||||||||
| api_key = get_api_key() | ||||||||||||||||
| if api_key: | ||||||||||||||||
| env_dict["RUNPOD_API_KEY"] = api_key | ||||||||||||||||
| log.debug( | ||||||||||||||||
| f"{self.name}: Injected RUNPOD_API_KEY for remote calls " | ||||||||||||||||
| f"(makes_remote_calls=True)" | ||||||||||||||||
| ) | ||||||||||||||||
| else: | ||||||||||||||||
| log.warning( | ||||||||||||||||
| f"{self.name}: makes_remote_calls=True but RUNPOD_API_KEY not set. " | ||||||||||||||||
| f"Remote calls to other endpoints will fail." | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| self.env = env_dict | ||||||||||||||||
|
|
||||||||||||||||
| # Inject module path for load-balanced endpoints | ||||||||||||||||
| elif self.type == ServerlessType.LB: | ||||||||||||||||
| env_dict = self.env or {} | ||||||||||||||||
|
|
||||||||||||||||
| module_path = self._get_module_path() | ||||||||||||||||
| if module_path and "FLASH_MODULE_PATH" not in env_dict: | ||||||||||||||||
| env_dict["FLASH_MODULE_PATH"] = module_path | ||||||||||||||||
| log.debug(f"{self.name}: Injected FLASH_MODULE_PATH={module_path}") | ||||||||||||||||
|
|
||||||||||||||||
| self.env = env_dict | ||||||||||||||||
| self._inject_runtime_template_vars() | ||||||||||||||||
|
|
||||||||||||||||
| # Ensure network volume is deployed first | ||||||||||||||||
| await self._ensure_network_volume_deployed() | ||||||||||||||||
|
|
@@ -764,8 +791,29 @@ async def update(self, new_config: "ServerlessResource") -> "ServerlessResource" | |||||||||||||||
|
|
||||||||||||||||
| if new_config.template: | ||||||||||||||||
| if resolved_template_id: | ||||||||||||||||
| # Skip env in the template payload when the user's env | ||||||||||||||||
| # hasn't changed. This lets the platform keep vars it | ||||||||||||||||
| # injected (e.g. PORT, PORT_HEALTH on LB endpoints) | ||||||||||||||||
| # and avoids a spurious rolling release. | ||||||||||||||||
| # | ||||||||||||||||
| # Also check template.env: if env is empty but the | ||||||||||||||||
| # caller provided explicit template env entries, those | ||||||||||||||||
| # must not be silently dropped. | ||||||||||||||||
| env_unchanged = self.env == new_config.env | ||||||||||||||||
| has_explicit_template_env = ( | ||||||||||||||||
| not new_config.env and new_config.template.env is not None | ||||||||||||||||
|
Comment on lines
+803
to
+804
|
||||||||||||||||
| has_explicit_template_env = ( | |
| not new_config.env and new_config.template.env is not None | |
| template_fields_set = getattr( | |
| new_config.template, "__pydantic_fields_set__", set() | |
| ) | |
| has_explicit_template_env = ( | |
| not new_config.env and "env" in template_fields_set |
Uh oh!
There was an error while loading. Please reload this page.