-
Notifications
You must be signed in to change notification settings - Fork 2
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 start agent balance thresholds #154
Conversation
truemiller
commented
May 29, 2024
•
edited
Loading
edited
- start agent button was disabled in some states where it shouldn't be
if (!services) return false; | ||
|
||
// deployment statuses where agent should not be deployed | ||
// if (serviceStatus === DeploymentStatus.DEPLOYED) return false; // condition already checked above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this commented code line? if yes, don't understand how it's checked above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the condition is covered on L179:
if (serviceStatus === DeploymentStatus.DEPLOYED) { |
had left the comment for context around why it's not included in the statuses where agent should not be deployable
can remove though, let me know 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good, but it might be personal preference, I haven't commonly seen IIFE being used. Also, regarding condition, can be added just to avoid confusion, but it is up to you.
function checkDeployability() {
// case where required values are undefined (not fetched from the server)
if (totalEthBalance === undefined || totalOlasBalance === undefined || !services) {
return false;
}
// deployment statuses where agent should not be deployed
if (
serviceStatus === DeploymentStatus.DEPLOYED ||
serviceStatus === DeploymentStatus.DEPLOYING ||
serviceStatus === DeploymentStatus.STOPPING
) {
return false;
}
// case where service exists & user has initial funded
if (services[0] && storeState?.isInitialFunded) {
return totalOlasBalance >= requiredOlas; // at present agent will always require staked/bonded OLAS
}
return totalOlasBalance >= requiredOlas && totalEthBalance > requiredGas;
}
const isDeployable = checkDeployability();
if (!isDeployable) {
return (
<Button type="default" size="large" disabled>
Start agent
</Button>
);
}
if (!services) return false; | ||
|
||
// deployment statuses where agent should not be deployed | ||
// if (serviceStatus === DeploymentStatus.DEPLOYED) return false; // condition already checked above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good, but it might be personal preference, I haven't commonly seen IIFE being used. Also, regarding condition, can be added just to avoid confusion, but it is up to you.
function checkDeployability() {
// case where required values are undefined (not fetched from the server)
if (totalEthBalance === undefined || totalOlasBalance === undefined || !services) {
return false;
}
// deployment statuses where agent should not be deployed
if (
serviceStatus === DeploymentStatus.DEPLOYED ||
serviceStatus === DeploymentStatus.DEPLOYING ||
serviceStatus === DeploymentStatus.STOPPING
) {
return false;
}
// case where service exists & user has initial funded
if (services[0] && storeState?.isInitialFunded) {
return totalOlasBalance >= requiredOlas; // at present agent will always require staked/bonded OLAS
}
return totalOlasBalance >= requiredOlas && totalEthBalance > requiredGas;
}
const isDeployable = checkDeployability();
if (!isDeployable) {
return (
<Button type="default" size="large" disabled>
Start agent
</Button>
);
}