-
Notifications
You must be signed in to change notification settings - Fork 11
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
ms2/engine unified endpoints #434
Conversation
0d64c4c
to
09229fe
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
src/management-system-v2/app/(dashboard)/[environmentId]/executions/[processId]/page.tsx
Outdated
Show resolved
Hide resolved
...system-v2/app/(dashboard)/[environmentId]/executions/[processId]/process-deployment-view.tsx
Show resolved
Hide resolved
…tions/[processId]/page.tsx Co-authored-by: Kai Rohwer <rohwer.kai@icloud.com>
…s/proceed into ms2/engine-unified-endpoints
✅ Successfully created Preview Deployment. |
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 and works great. There is a small issue that does not affect the functionality too much but should be fixed sometime in the future.
function getLatestVersion(process: DeployedProcessInfo) { | ||
let latest = process.versions.length - 1; | ||
for (let i = process.versions.length - 2; i >= 0; i--) { | ||
if (process.versions[i].version > process.versions[latest].version) latest = i; |
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 does not work with the new version info that contains versionId instead of version which is also not usable to infer which version is the newest.
for (const deployment of deployments) { | ||
let latestVesrionIdx = deployment.versions.length - 1; | ||
for (let i = deployment.versions.length - 2; i >= 0; i--) { | ||
if (deployment.versions[i].version > deployment.versions[latestVesrionIdx].version) |
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.
Same problem as in the instance view. I think this will not work correctly since there should not be a version
entry in the objects inside versions
anymore.
const ExecutionsPage = async ({ params }: { params: { environmentId: string } }) => { | ||
function getDeploymentNames(deployments: DeployedProcessInfo[]) { | ||
for (const deployment of deployments) { | ||
let latestVesrionIdx = deployment.versions.length - 1; |
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.
Small typo
@@ -202,7 +224,6 @@ export type VersionInfo = { | |||
versionName: string; |
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 think you need to replace the version
entry above with versionId
|
||
// Endpoints | ||
type EndpointSchema = typeof import('./endpoints.json'); | ||
type Endpoints = EndpointSchema; |
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.
Is this needed because of the json import or could you just do type Endpoints = typeof import('./endpoints.json');
without the EndpointSchema assignment?
Summary
Engine
which includes mqtt/http and space/PROCEED engines.Engine
and makes the appropriate request.There are still TODOS in the code, but the basic structure is there.