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

Fix/update service deletion #19

Merged
merged 7 commits into from
Feb 26, 2024
Merged

Fix/update service deletion #19

merged 7 commits into from
Feb 26, 2024

Conversation

truemiller
Copy link
Collaborator

  • update service deletion logic; currently deleted services contain active:false until deletion is resolved on backend side
  • some optimizations to only update a single service, rather than polling for all services when we need to update state

Copy link
Collaborator

@mohandast52 mohandast52 left a comment

Choose a reason for hiding this comment

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

LGTM


// Set agent funding requirements
if (_service.chain_data?.instances) {
if (service.chain_data?.instances) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You might consider adding it to a new variable since it is used in more than one place.

Suggested change
if (service.chain_data?.instances) {
const instances = service.chain_data?.instances;
if (instances) {

setIsStaking(true);
setSpawnScreenState(nextPage);
const service: Service | undefined = await create(true);
if (!service) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not a big deal, but we could reverse this for better readability.

if (service) { // positive... }
else { // error... }


await updateServiceState(service.hash).catch(() =>
message.error('Failed to update service state'),
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

usage of await and catch 🙈, can avoid any one, preferably catch.

const newServices = [...prev];
newServices[index] = service;
return newServices;
}),
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we want to catch and log an error?

@truemiller truemiller merged commit 86d5fe0 into main Feb 26, 2024
2 checks passed
@truemiller truemiller deleted the fix/update-service-deletion branch February 26, 2024 10:56
0xArdi pushed a commit that referenced this pull request Oct 17, 2024
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.

2 participants