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

refactor: improve performance of checking for offline deployments #1718

Conversation

MohamedElmdary
Copy link
Member

@MohamedElmdary MohamedElmdary commented Dec 18, 2023

Description

checking for offline deployments was loading all nodes on grid

Changes

Load only nodes which used in my contracts

Related Issues

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstrings
  • Screenshots/Video attached (needed for UI changes)

Copy link
Contributor

@amiraabouhadid amiraabouhadid left a comment

Choose a reason for hiding this comment

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

verified
image

Copy link
Contributor

@zaelgohary zaelgohary left a comment

Choose a reason for hiding this comment

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

I don't think we need this function anymore.

export async function getAllNodes(grid: GridClient | null, options?: NodeFilters): Promise<NodeInfo[] | number[]> {
const isFlat = options?.flat || false;
const isLastPage = requestPageNumber.value === 0;
if (isLastPage) {
if (isFlat) {
return offlineNodes.value.map(node => node.nodeId);
}
return offlineNodes.value;
} else {
const offNodes: NodeInfo[] = await grid!.nodes.filter({
...options,
// status: NodeStatus.down,
page: requestPageNumber.value,
});
if (offNodes.length) {
offlineNodes.value.push(...offNodes);
requestPageNumber.value += 1;
} else {
// needs to be last page.
requestPageNumber.value = 0;
}
}
return getAllNodes(grid, options);
}

@zaelgohary
Copy link
Contributor

Great fix ya @MohamedElmdary!

@AhmedHanafy725 AhmedHanafy725 merged commit 60a8ebf into development_rewrite_node_selector Dec 19, 2023
@AhmedHanafy725 AhmedHanafy725 deleted the development_rewrite_node_selector_notifications branch December 19, 2023 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants