feat(ws): stop using mocks for frontend get workspaces #188

yehudit1987 · 2025-01-30T10:29:53Z

This PR resolve issue #142

ederign · 2025-02-05T19:28:59Z

@paulovmr would mind to take a look on this one?

ederign · 2025-02-05T19:29:13Z

/assign @paulovmr

thesuperzapper · 2025-02-20T16:50:09Z

/ok-to-test

thesuperzapper · 2025-03-07T02:20:58Z

@yehudit1987 this can be rebased now that #213 is merged.

The backend changes from this PR should no longer be needed, but the frontend will need to understand the new "services" part of the API from that PR.

paulovmr

Hi @yehudit1987 , thanks for the PR! I've made a partial review, and will test it as well after the backend PR is merged and this one is rebased. I've left a few comments inline. Also:

A .gitignore entry was added and already merged in the notebooks-v2 branch to avoid the workspaces/frontend/src/__tests__/cypress/cypress/downloads/downloads.htm file to be versioned. Your PR is trying to add the file. My guess is that because when you opened the PR the .gitignore entry did not exist, the download.htm file was already added to your commit, so you might need to remove it manually in your branch.

paulovmr · 2025-03-12T15:16:31Z

workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspace.mock.ts

+  podConfigDisplayName: string,
+  pvcName: string,
+): {
+  name: string;


Could this return type be replaced by the Workspace type directly?

paulovmr · 2025-03-12T15:24:59Z

workspaces/frontend/src/app/pages/Workspaces/Details/WorkspaceDetailsActivity.tsx

@@ -16,7 +16,7 @@ type WorkspaceDetailsActivityProps = {
 export const WorkspaceDetailsActivity: React.FunctionComponent<WorkspaceDetailsActivityProps> = ({
  workspace,
 }) => {
-  const { activity, pauseTime, pendingRestart } = workspace.status;
+  const { activity, pausedTime: pauseTime, pendingRestart } = workspace;


Just to keep the consistency, please consider keeping all usages as pausedTime (also label and usage in lines 38 to 40).

Suggested change

const { activity, pausedTime: pauseTime, pendingRestart } = workspace;

const { activity, pausedTime, pendingRestart } = workspace;

paulovmr · 2025-03-12T15:52:32Z

workspaces/frontend/src/app/pages/Workspaces/Details/WorkspaceDetailsOverview.tsx

    </DescriptionListGroup>
    <Divider />
    <DescriptionListGroup>
      <DescriptionListTerm>Labels</DescriptionListTerm>
      <DescriptionListDescription>
-        {workspace.podTemplate.podMetadata.labels.join(', ')}
+        {workspace.podTemplate.options.podConfig.current.labels.length > 0


I believe we could leverage PatternFly Label component to better list the labels here. Since it is out of the scope of this PR, I've created a different issue for it: #232

paulovmr · 2025-03-12T15:56:22Z

workspaces/frontend/src/app/pages/Workspaces/Details/WorkspaceDetailsOverview.tsx

      </DescriptionListDescription>
    </DescriptionListGroup>
    <Divider />
    <DescriptionListGroup>
      <DescriptionListTerm>Pod config</DescriptionListTerm>
-      <DescriptionListDescription>{workspace.options.podConfig}</DescriptionListDescription>
+      <DescriptionListDescription>
+        {JSON.stringify(workspace.podTemplate.options.podConfig, null, 2)}


Suggested change

{JSON.stringify(workspace.podTemplate.options.podConfig, null, 2)}

{workspace.podTemplate.options.podConfig.current.displayName}

Signed-off-by: Yehudit Kerido <[email protected]>

paulovmr

/lgtm

google-oss-prow · 2025-03-17T21:03:26Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: paulovmr
Once this PR has been reviewed and has the lgtm label, please assign ederign for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

workspaces/frontend/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ederign

@paulovmr @yehudit1987 my suggestion is to hold this until we:

Have a better controller installation/setup with feat: setup tilt for easier local controller development #220
Figure out where we should mock. I suggest we discuss mocking this on BFF layer to avoid dependency on controller for front-end development. After we finish a feature we can always test it integrated (ideally with automated tests and at least with tilt)

ederign · 2025-03-19T18:36:48Z

/hold

ederign · 2025-05-06T15:59:42Z

@paulovmr @yehudit1987 if this is still planned, would mind to rebase it?

caponetto · 2025-05-06T16:23:50Z

Please hold this merge until the PR for #269 is integrated, if possible. We have many overlapping changes.

yehudit1987 · 2025-05-07T09:17:58Z

@paulovmr @yehudit1987 if this is still planned, would mind to rebase it?

@ederign Are we planning to merge that?, rebase involves reviewing so many conflicts so I prefer to do it once and only when it's relevant.

paulovmr · 2025-05-07T09:27:48Z

@yehudit1987 Let's wait for a little bit. Once all PRs that will conflict with yours are merged we can do the rebase.

google-oss-prow bot requested review from ederign and thesuperzapper January 30, 2025 10:29

google-oss-prow bot added the size/XL label Jan 30, 2025

yehudit1987 force-pushed the fetch-workspace-list branch from 5feeff6 to 6aac5ce Compare February 2, 2025 08:24

yehudit1987 changed the title ~~Fetch workspace list~~ feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Feb 3, 2025

yehudit1987 force-pushed the fetch-workspace-list branch from 9ba35a6 to d356044 Compare February 5, 2025 07:11

google-oss-prow bot assigned paulovmr Feb 5, 2025

yehudit1987 force-pushed the fetch-workspace-list branch 2 times, most recently from ebfda26 to 9ab07f3 Compare February 12, 2025 13:34

google-oss-prow bot added the ok-to-test label Feb 20, 2025

thesuperzapper changed the title ~~feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces~~ feat(ws): stop using mocks for frontend get workspaces Feb 27, 2025

yehudit1987 marked this pull request as draft February 27, 2025 16:31

google-oss-prow bot added the do-not-merge/work-in-progress label Feb 27, 2025

thesuperzapper mentioned this pull request Mar 7, 2025

feat(ws): add http service paths to WS get on backend #213

Merged

yehudit1987 force-pushed the fetch-workspace-list branch from c15e8e5 to 6814730 Compare March 9, 2025 07:23

yehudit1987 marked this pull request as ready for review March 12, 2025 12:36

google-oss-prow bot removed the do-not-merge/work-in-progress label Mar 12, 2025

paulovmr mentioned this pull request Mar 12, 2025

Add labels list to Workspaces details #232

Open

4 tasks

paulovmr suggested changes Mar 12, 2025

View reviewed changes

Yehudit Kerido added 7 commits March 17, 2025 08:33

reat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

8e7b47a

Signed-off-by: Yehudit Kerido <[email protected]>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

faeef0d

Signed-off-by: Yehudit Kerido <[email protected]>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

20b7912

Signed-off-by: Yehudit Kerido <[email protected]>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

e065649

Signed-off-by: Yehudit Kerido <[email protected]>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

7dc227e

Signed-off-by: Yehudit Kerido <[email protected]>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

ee9ea62

Signed-off-by: Yehudit Kerido <[email protected]>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

b22f179

Signed-off-by: Yehudit Kerido <[email protected]>

Yehudit Kerido added 4 commits March 17, 2025 08:33

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

309a134

Signed-off-by: Yehudit Kerido <[email protected]>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

dc2c179

Signed-off-by: Yehudit Kerido <[email protected]>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

df044e9

Signed-off-by: Yehudit Kerido <[email protected]>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces

bfade6d

Signed-off-by: Yehudit Kerido <[email protected]>

yehudit1987 force-pushed the fetch-workspace-list branch from 61c4593 to bfade6d Compare March 17, 2025 06:34

Yehudit Kerido added 2 commits March 17, 2025 11:58

fix cypress tests

6a1db5f

Signed-off-by: Yehudit Kerido <[email protected]>

fix cypress tests

1e27798

Signed-off-by: Yehudit Kerido <[email protected]>

paulovmr approved these changes Mar 17, 2025

View reviewed changes

google-oss-prow bot added the lgtm label Mar 17, 2025

ederign suggested changes Mar 19, 2025

View reviewed changes

google-oss-prow bot added the do-not-merge/hold label Mar 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ws): stop using mocks for frontend get workspaces #188

feat(ws): stop using mocks for frontend get workspaces #188

yehudit1987 commented Jan 30, 2025

ederign commented Feb 5, 2025

ederign commented Feb 5, 2025

thesuperzapper commented Feb 20, 2025

thesuperzapper commented Mar 7, 2025

paulovmr left a comment

paulovmr Mar 12, 2025

paulovmr Mar 12, 2025

paulovmr Mar 12, 2025

paulovmr Mar 12, 2025

paulovmr left a comment

google-oss-prow bot commented Mar 17, 2025

ederign left a comment •

edited

Loading

ederign commented Mar 19, 2025

ederign commented May 6, 2025

caponetto commented May 6, 2025

yehudit1987 commented May 7, 2025 •

edited

Loading

paulovmr commented May 7, 2025

	const { activity, pausedTime: pauseTime, pendingRestart } = workspace;
	const { activity, pausedTime, pendingRestart } = workspace;

	{JSON.stringify(workspace.podTemplate.options.podConfig, null, 2)}
	{workspace.podTemplate.options.podConfig.current.displayName}

feat(ws): stop using mocks for frontend get workspaces #188

Are you sure you want to change the base?

feat(ws): stop using mocks for frontend get workspaces #188

Conversation

yehudit1987 commented Jan 30, 2025

ederign commented Feb 5, 2025

ederign commented Feb 5, 2025

thesuperzapper commented Feb 20, 2025

thesuperzapper commented Mar 7, 2025

paulovmr left a comment

Choose a reason for hiding this comment

paulovmr Mar 12, 2025

Choose a reason for hiding this comment

paulovmr Mar 12, 2025

Choose a reason for hiding this comment

paulovmr Mar 12, 2025

Choose a reason for hiding this comment

paulovmr Mar 12, 2025

Choose a reason for hiding this comment

paulovmr left a comment

Choose a reason for hiding this comment

google-oss-prow bot commented Mar 17, 2025

ederign left a comment • edited Loading

Choose a reason for hiding this comment

ederign commented Mar 19, 2025

ederign commented May 6, 2025

caponetto commented May 6, 2025

yehudit1987 commented May 7, 2025 • edited Loading

paulovmr commented May 7, 2025

ederign left a comment •

edited

Loading

yehudit1987 commented May 7, 2025 •

edited

Loading