Skip to content
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

Catch and display load errors for WorkloadsOverview #4393

Merged
merged 2 commits into from
Nov 22, 2021
Merged

Conversation

Nokel81
Copy link
Collaborator

@Nokel81 Nokel81 commented Nov 19, 2021

  • Move the select filter to the top level since that is where the
    subscribe call is

Signed-off-by: Sebastian Malton sebastian@malton.name

fixes #4344

- Move the select filter to the top level since that is where the
  subscribe call is

Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81 Nokel81 added the bug Something isn't working label Nov 19, 2021
@Nokel81 Nokel81 requested a review from a team as a code owner November 19, 2021 16:11
@Nokel81 Nokel81 requested review from nevalla and jakolehm and removed request for a team November 19, 2021 16:11
Copy link
Contributor

@jim-docker jim-docker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something fishy:
Screen Shot 2021-11-19 at 2 42 21 PM

User does not have namespace list permission, accessible namespaces are set to default, freddy, and garbage. User has access only to default, garbage namespace does not exist.

With just default selected I would not expect any warnings

Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81 Nokel81 requested a review from jim-docker November 19, 2021 20:53
@Nokel81
Copy link
Collaborator Author

Nokel81 commented Nov 19, 2021

@jim-docker Good catch, thanks. I also fixed the same issue on the <KubeObjectListLayout /> because it exists there too.

Comment on lines +35 to +45
type RegisteredWorkloadsOverviewDetail = Required<WorkloadsOverviewDetailRegistration>;

export class WorkloadsOverviewDetailRegistry extends BaseRegistry<WorkloadsOverviewDetailRegistration, RegisteredWorkloadsOverviewDetail> {
getItems() {
const items = super.getItems();
return orderBy(super.getItems(), "priority", "desc");
}

protected getRegisteredItem(item: WorkloadsOverviewDetailRegistration): RegisteredWorkloadsOverviewDetail {
const { priority = 50, ...rest } = item;

return items.sort((a, b) => (b.priority ?? 50) - (a.priority ?? 50));
return { priority, ...rest };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks unrelated to the issue, did you test that it still works?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I did. With the kube-resource-map extension installed (and the default overview doesn't have priority defined) the order is the exact same as before.

namespaces: clusterContext.contextNamespaces,
onLoadFailure: error => this.loadErrors.push(String(error)),
}),
reaction(() => clusterContext.contextNamespaces.slice(), () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't it just be () => clusterContext.contextNamespaces?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because then it only tracks assignments to the clusterContext.contextNamespaces field itself, and not internal changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. You have to "dot" into it. Ugh.

@Nokel81 Nokel81 merged commit 78a4e2a into master Nov 22, 2021
@Nokel81 Nokel81 deleted the issue-4344 branch November 22, 2021 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

One inaccessible namespace blocks Workloads Overview for all namespaces
2 participants