RHSTOR-6195: Bucket - list page #1598

GowthamShanmugam · 2024-09-26T13:30:32Z

https://issues.redhat.com/browse/RHSTOR-6195
ToDo bucket sync button after merging: #1591

openshift-ci-robot · 2024-09-26T13:30:37Z

@GowthamShanmugam: This pull request references RHSTOR-6195 which is a valid jira issue.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2024-09-26T13:30:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GowthamShanmugam
Once this PR has been reviewed and has the lgtm label, please assign cloudbehl 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:

OWNERS

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

openshift-ci-robot · 2024-09-26T13:33:20Z

@GowthamShanmugam: This pull request references RHSTOR-6195 which is a valid jira issue.

In response to this:

https://issues.redhat.com/browse/RHSTOR-6195

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

GowthamShanmugam · 2024-09-26T14:00:32Z

Signed-off-by: Gowtham Shanmugasundaram <[email protected]>

openshift-ci-robot · 2024-09-26T14:07:12Z

@GowthamShanmugam: This pull request references RHSTOR-6195 which is a valid jira issue.

In response to this:

https://issues.redhat.com/browse/RHSTOR-6195
ToDo bucket sync button after merging: #1591

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2024-09-26T15:51:52Z

@GowthamShanmugam: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/odf-console-e2e-aws	`8b665ba`	link	true	`/test odf-console-e2e-aws`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

SanjalKatiyar · 2024-09-30T11:17:50Z

locales/en/plugin__odf-console.json

+  "No buckets found": "No buckets found",
+  "Create bucket": "Create bucket",
+  "Create and manage your buckets": "Create and manage your buckets",
+  "Navigate through your buckets effortlessly. View the contents of your S3-managed and Openshift-managed buckets, making it easy to locate and inspect objects. Track and manage object versions and deleted items. View and restore previous versions of objects and access a history of deleted items, ensuring you can recover or review data as needed.": "Navigate through your buckets effortlessly. View the contents of your S3-managed and Openshift-managed buckets, making it easy to locate and inspect objects. Track and manage object versions and deleted items. View and restore previous versions of objects and access a history of deleted items, ensuring you can recover or review data as needed.",


not in phase 1

Suggested change

"Navigate through your buckets effortlessly. View the contents of your S3-managed and Openshift-managed buckets, making it easy to locate and inspect objects. Track and manage object versions and deleted items. View and restore previous versions of objects and access a history of deleted items, ensuring you can recover or review data as needed.": "Navigate through your buckets effortlessly. View the contents of your S3-managed and Openshift-managed buckets, making it easy to locate and inspect objects. Track and manage object versions and deleted items. View and restore previous versions of objects and access a history of deleted items, ensuring you can recover or review data as needed.",

"Navigate through your buckets effortlessly. View the contents of your S3-managed and Openshift-managed buckets, making it easy to locate and inspect objects.": "Navigate through your buckets effortlessly. View the contents of your S3-managed and Openshift-managed buckets, making it easy to locate and inspect objects.",

SanjalKatiyar · 2024-09-30T11:18:27Z

locales/en/plugin__odf-console.json

@@ -1331,6 +1337,10 @@
  "No resources available": "No resources available",
  "Select {{resourceLabel}}": "Select {{resourceLabel}}",
  "Error Loading": "Error Loading",
+  "Loading Empty Page": "Loading Empty Page",


or it should be capital only ??

Suggested change

"Loading Empty Page": "Loading Empty Page",

"Loading empty page": "Loading Empty Page",

SanjalKatiyar · 2024-09-30T11:23:58Z

no need for extra "bucket" directory here: /s3-browser/bucket/buckets-list-page/bucketsListPage.tsx...
we can directly do packages/odf/components/s3-browser/buckets-list-page/bucketsListPage.tsx

Also, bucketsListPage.tsx should be BucketsListPage.tsx...

IIRC that's what we decided, cc @alfonsomthd to keep me honest.

SanjalKatiyar · 2024-09-30T11:37:14Z

packages/odf/components/s3-browser/bucket/buckets-list-page/bucketsListPage.tsx

+  const [buckets, filteredData, onFilterChange] = useListPageFilter(
+    convertS3ListBucketOutputToK8sCompatible(data)
+  );
+  const [favorites, setFavorites] = useUserSettings<BucketFavoritesType>(


useUserSettings first stores items in a ConfigMap then as a fallback to a local-storage...

for our use case there can be 100s of buckets (probably 1000), so might be too much info for a single CM ??

should we use useUserSettingsLocalStorage only ??

cc @GowthamShanmugam @bipuladh @alfonsomthd wdyt ?

The data stored in a ConfigMap cannot exceed 1 MiB. Provided that even 1000 buckets (max buckets an accoun can have) do not reach that threshold, then it's a matter of consensus. Otherwise we should definitely not store it in a ConfigMap.

it's a global CM (shared by OCP and all its dynamic plugins), lot other things are stored there...

also, in future when we support more accounts we definitely will have to revert to local storage way only... because CM is still global...

it's a global CM (shared by OCP and all its dynamic plugins), lot other things are stored there...

Then definitely it's not a safe place to store that data.

Ok! I thought the bucket count wouldn't cross more than 200, so I decided to use the same configMap. And the user won't select all 200 as fav.

But local storage will be a terrible user experience, The user will see different fav in different browsers. And what is they selected 500 buckets as fav and other browser displays nothing

make sense !!
let's confirm with OCP team on what they think.

Also, @GowthamShanmugam just a reminder that on a bucket deletion, we need to update this CM/LocalStorage as well.

SanjalKatiyar · 2024-09-30T11:39:59Z

packages/odf/components/s3-browser/bucket/buckets-list-page/bucketsListPage.tsx

+    noobaaS3.listBuckets()
+  );
+  const [buckets, filteredData, onFilterChange] = useListPageFilter(
+    convertS3ListBucketOutputToK8sCompatible(data)


nits: name too long:P

SanjalKatiyar · 2024-09-30T11:41:00Z

packages/odf/components/s3-browser/bucket/buckets-list-page/bucketsListPage.tsx

+type BucketFavoritesType = {
+  [key in string]: boolean;
+};


better to have a Set with all the buckets which are marked as favourite...

no need to store the once which are false...

SanjalKatiyar · 2024-09-30T11:41:32Z

packages/odf/components/s3-browser/bucket/buckets-list-page/bucketsListPage.tsx

+    true
+  );
+
+  const isLoadeddWOAnyError = !isLoading && !error;


Suggested change

const isLoadeddWOAnyError = !isLoading && !error;

const isLoadedWOAnyError = !isLoading && !error;

SanjalKatiyar · 2024-09-30T11:50:03Z

packages/odf/components/s3-browser/bucket/buckets-list-page/bucketsListPage.tsx

+  const { Name: name, CreationDate: creationDate } = bucket.apiResponse;
+  const { favorites, setFavorites }: RowExtraPropsType = extraProps;
+
+  const onSetFavorite = React.useCallback(


no need for useCallback...
onFavorite ref below is already changing on re-render, saving onSetFavorite won't do any good...

SanjalKatiyar · 2024-09-30T11:55:10Z

packages/shared/src/table/composable-table.tsx

@@ -67,6 +71,7 @@ export const ComposableTable: ComposableTableProps = <
        translate={null}
        aria-label="Composable table"
        className="pf-v5-u-mt-md"
+        variant={isCompact ? TableVariant.compact : null}


totally optional comment:

Suggested change

variant={isCompact ? TableVariant.compact : null}

{...(isCompact ? {variant: ableVariant.compact} : {})}

SanjalKatiyar · 2024-09-30T11:58:39Z

plugins/odf/console-extensions.json

-    },
-    "flags": {
-      "required": ["MCG"]


no need to remove this IMO...
why are we removing this ??

ok! i accidently removed this

SanjalKatiyar · 2024-09-30T11:59:09Z

plugins/odf/console-extensions.json

-  {
-    "type": "odf.horizontalNav/tab",
-    "properties": {
-      "id": "objectbuckets",
-      "name": "%plugin__odf-console~Object Buckets%",
-      "contextId": "odf-object-service",
-      "after": "namespace-store",
-      "href": "objectbucket.io~v1alpha1~ObjectBucket",
-      "component": {
-        "$codeRef": "ob.ObjectBucketListPage"
-      }
-    },
-    "flags": {
-      "required": ["MCG"]
-    }
-  },


we still need it for non-mcg-standalone deployments...

only time we don't need is for mcg-standalone specifically...

Do you mean this one: https://github.com/red-hat-storage/odf-console/blob/master/plugins/odf/console-extensions.json#L607?

I still keeping the RGW case

SanjalKatiyar · 2024-09-30T12:08:03Z

packages/shared/src/list-page/paginated-list-page.tsx

@@ -20,21 +24,66 @@ const COUNT_PER_PAGE_NUMBER = 10;
 export type PaginatedListPageProps = {
  countPerPage?: number;
  filteredData: K8sResourceCommon[];
-  CreateButton: React.FC<unknown>;


it's a shared component, do we really need to change the name ??
need to check the impact (if any) on Fusion side, if we are planning a rename...

No problem. for now, I will keep the name CreateButton. I renamed this to make this paginated component generic to create any kind of list page between MCO and ODF.

openshift-ci-robot added the jira/valid-reference label Sep 26, 2024

openshift-merge-robot added the needs-rebase label Sep 26, 2024

GowthamShanmugam force-pushed the RHSTOR-6195 branch from 03573e8 to 8adf721 Compare September 26, 2024 13:32

openshift-merge-robot removed the needs-rebase label Sep 26, 2024

GowthamShanmugam force-pushed the RHSTOR-6195 branch from 8adf721 to a3a6edf Compare September 26, 2024 13:50

RHSTOR-6195: Bucket - list page

8b665ba

Signed-off-by: Gowtham Shanmugasundaram <[email protected]>

GowthamShanmugam force-pushed the RHSTOR-6195 branch from a3a6edf to 8b665ba Compare September 26, 2024 14:03

SanjalKatiyar reviewed Sep 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RHSTOR-6195: Bucket - list page #1598

RHSTOR-6195: Bucket - list page #1598

GowthamShanmugam commented Sep 26, 2024 •

edited

Loading

openshift-ci-robot commented Sep 26, 2024 •

edited by openshift-ci bot

Loading

openshift-ci bot commented Sep 26, 2024

openshift-ci-robot commented Sep 26, 2024 •

edited by openshift-ci bot

Loading

GowthamShanmugam commented Sep 26, 2024 •

edited

Loading

openshift-ci-robot commented Sep 26, 2024 •

edited by openshift-ci bot

Loading

openshift-ci bot commented Sep 26, 2024

SanjalKatiyar Sep 30, 2024

GowthamShanmugam Sep 30, 2024

SanjalKatiyar Sep 30, 2024

SanjalKatiyar commented Sep 30, 2024 •

edited

Loading

SanjalKatiyar Sep 30, 2024

alfonsomthd Sep 30, 2024 •

edited

Loading

SanjalKatiyar Sep 30, 2024

SanjalKatiyar Sep 30, 2024

alfonsomthd Sep 30, 2024

GowthamShanmugam Sep 30, 2024

GowthamShanmugam Sep 30, 2024

SanjalKatiyar Sep 30, 2024

SanjalKatiyar Sep 30, 2024

GowthamShanmugam Sep 30, 2024

SanjalKatiyar Sep 30, 2024

SanjalKatiyar Sep 30, 2024

SanjalKatiyar Sep 30, 2024

SanjalKatiyar Sep 30, 2024

SanjalKatiyar Sep 30, 2024

GowthamShanmugam Sep 30, 2024

SanjalKatiyar Sep 30, 2024

SanjalKatiyar Sep 30, 2024

GowthamShanmugam Sep 30, 2024

SanjalKatiyar Sep 30, 2024

GowthamShanmugam Sep 30, 2024

	"Loading Empty Page": "Loading Empty Page",
	"Loading empty page": "Loading Empty Page",

	const isLoadeddWOAnyError = !isLoading && !error;
	const isLoadedWOAnyError = !isLoading && !error;

	variant={isCompact ? TableVariant.compact : null}
	{...(isCompact ? {variant: ableVariant.compact} : {})}

RHSTOR-6195: Bucket - list page #1598

Are you sure you want to change the base?

RHSTOR-6195: Bucket - list page #1598

Conversation

GowthamShanmugam commented Sep 26, 2024 • edited Loading

openshift-ci-robot commented Sep 26, 2024 • edited by openshift-ci bot Loading

openshift-ci bot commented Sep 26, 2024

openshift-ci-robot commented Sep 26, 2024 • edited by openshift-ci bot Loading

GowthamShanmugam commented Sep 26, 2024 • edited Loading

openshift-ci-robot commented Sep 26, 2024 • edited by openshift-ci bot Loading

openshift-ci bot commented Sep 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SanjalKatiyar commented Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

alfonsomthd Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GowthamShanmugam commented Sep 26, 2024 •

edited

Loading

openshift-ci-robot commented Sep 26, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Sep 26, 2024 •

edited by openshift-ci bot

Loading

GowthamShanmugam commented Sep 26, 2024 •

edited

Loading

openshift-ci-robot commented Sep 26, 2024 •

edited by openshift-ci bot

Loading

SanjalKatiyar commented Sep 30, 2024 •

edited

Loading

alfonsomthd Sep 30, 2024 •

edited

Loading