Repository navigation
Conversation
| if not cpu or any(not isinstance(c, str) or not c.strip() for c in cpu): | ||
| raise InvalidResourceError("cpu must contain non-empty instance ids") | ||
| supported = {flavor.value for flavor in CpuInstanceType} | ||
| if unsupported := set(cpu) - supported: |
There was a problem hiding this comment.
CpuInstanceType only lists cpu3g, cpu3c and cpu5c at 1 to 8 vCPUs, so @app.queue(cpu="cpu5g-4-16") or cpu3c-16-32 now fails at import with unknown cpu, where it deployed before. cpu5g is real elsewhere in the SDK: rp endpoint --cpu cpu5g, rp pod --cpu cpu5g, test_api_client.py:380 (cpu5g-4-16), and the note in datacenter.py that cpu5c/cpu5g are stocked in EU-RO-1. Since the backend validates instance ids anyway, could this check the <flavor>-<vcpu>-<mem> shape (the same pattern _cpu_config parses) instead of a fixed list, or at least add cpu5g and warn on unknown values instead of raising?
| rollback_errors.append( | ||
| f"build: {str(restore_exc) or type(restore_exc).__name__}" | ||
| ) | ||
| if creating and isinstance(exc, (aiohttp.ClientError, OSError)): |
There was a problem hiding this comment.
The lost-create lookup only runs for (aiohttp.ClientError, OSError). When the 60s total timeout fires while waiting for the response, aiohttp raises asyncio.TimeoutError. That is the builtin TimeoutError (an OSError) on 3.11+, but a separate class on 3.10, which is still in requires-python and the CI matrix. So on 3.10 a saveEndpoint create that succeeds server side after the client timed out is skipped. The user is told the changes were rolled back while the new endpoint keeps running, and billing if workersMin > 0. The tests raise the builtin TimeoutError, so they pass on 3.10 too. Adding asyncio.TimeoutError to the tuple, and a test that raises it, would close this.
| creating = False | ||
| log.info("saved endpoint %s (%s)", name, result["id"]) | ||
|
|
||
| except Exception as exc: |
There was a problem hiding this comment.
The handler is except Exception, and KeyboardInterrupt and asyncio.CancelledError are BaseExceptions. Under asyncio.run, Ctrl-C after the new build is activated and a couple of endpoints are saved leaves the new build live, endpoints partly updated and new ones created, with nothing rolled back or reported. That matches the behavior before this PR, so it isn't a regression, but the description says completed operations are completely rolled back. Should the rollback also run on cancellation (catch BaseException, run the undo under asyncio.shield, then re-raise), or should the description and the interrupt output point at rp flash env get instead?
| rollback_errors = [] | ||
| if activation_attempted and not environment_created: | ||
| try: | ||
| previous_build = environment.get("activeBuildId") |
There was a problem hiding this comment.
The build restore here and the endpoint restores below write the snapshot back without checking that the environment and endpoints still hold what this run wrote. If deploy A (say from CI) snapshots endpoint X, deploy B then updates X and activates its build, and A then fails, A restores X to its old snapshot and re-activates the previous build. B already reported success. The environment-delete path guards against this with the activeBuildId not in (None, build_id) check; could the build and endpoint restores do the same (skip when activeBuildId != build_id, or when the endpoint's RUNPOD_BUILD_ID is no longer this build) and report them as changed by another deploy?
When reconciling endpoint updates when deploying a flash app, if something fails, will completely roll back any completed operations (updates & creations) and then stop the deploy.
A recovered failure ends with:
Obsolete endpoints are removed after the deployment succeeds. If that cleanup fails, the deployed app stays in place and the error identifies the endpoint that could not be removed.
Also now runs a parser over everything before attempting the deploy to catch easy validation issues before getting caught mid deploy.
Fixes CON-2174