-
Notifications
You must be signed in to change notification settings - Fork 2
fix: Add environmentV2 repository to list new environment types #244
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
Conversation
| ids?: string[]; | ||
| partialName?: string; | ||
| skip: number; | ||
| take: number; |
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.
I've made the skip and take values required as they're required on the v2 endpoint.
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 introduces a new EnvironmentV2Repository class to support querying deployment environments using the v2 API endpoint, and includes an example demonstrating its usage.
- Added
EnvironmentV2Repositoryclass with alistmethod to query environments via the v2 API endpoint - Exported the new repository from the deploymentEnvironments module
- Created an example script showing how to use the new repository
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/features/deploymentEnvironments/index.ts | Exports the new EnvironmentV2Repository class |
| src/features/deploymentEnvironments/environmentV2Repository.ts | Implements the new repository class for v2 API endpoint |
| examples/src/environments/get-environments.ts | Demonstrates usage of the new repository |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
acc9024 to
4ad2f1c
Compare
4ad2f1c to
c7b3cdc
Compare
geofflamrock
left a comment
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.
Looks good, one thought for completeness of the environments v2 api but doesn't need a re-review unless you would like it.
| @@ -0,0 +1,25 @@ | |||
| import { Client, DeploymentEnvironmentV2, ResourceCollection, spaceScopedRoutePrefix } from "../.."; | |||
|
|
|||
| type EnvironmentV2RepositoryListArgs = { | |||
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 v2 endpoint supports optional filtering by an array of environment types using the type property, for completeness it's prob worth adding that as well.
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.
Good idea :)
Background 🌇
As part of our setup to support ephemeral environments, we have added a
DeploymentEnvironmentV2environment type which encompasses static deployment environments, parent environments and ephemeral environments.Our existing
environmentRepository's list function returns a list of the oldDeploymentEnvironmenttype representing only static deployment environments.What's this? 🌵
This PR adds a
environmentsV2Repositoryto work with theDeploymentEnvironmentV2type. At the moment, the only functionality we need from the new repository is alistendpoint.🚩 The
environmentRepositoryalso contains a few functions for working with theDeploymentEnvironmentV2. However a new repository is required to add functions whichenvironmentRepositoryinherits fromSpaceScopedBasicRepositorysuch aslist,create,del, etc. as these are all set up to return only the old environment type. (And changing these would be a huge breaking change.)Testing 🧪
When tested on a cloud instance using the
deploy-release-action(the requirements of which inspired this change) the newlistfunction successfully returned a list of environments including ephemeral environments.When tested locally using the example file, the new
listfunction returned items of all three environment types.How to review? 🔍
☑️ Anything I've missed?
Fixes[sc-127375]