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

Support for deleting deployments #15

Merged
merged 4 commits into from
Feb 21, 2024

Conversation

angrybayblade
Copy link
Contributor

No description provided.

chain_data?: ChainData;
ledger: LedgerConfig;
chain_data: ChainData;
active: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@truemiller only display the service with active set to true on the front end

Comment on lines +126 to +132
@_operate.command(name="prune")
def _prune(
home: Annotated[
Path, params.Directory(long_flag="--home", help="Home directory")
] = None,
) -> None:
"""Delete unused/cached data."""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dvilelaf When installing the operate app, we will install operate as a CLI tool as well so that users can run operate prune to delete the cache and unused deployments

Copy link
Collaborator

Choose a reason for hiding this comment

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

So we're requiring users to run a command? Not sure if that bodes well with the idea of the app. If users don't run this command, what will happen? Old deployments will just be there?

# except Exception as e:
# print(e)

old.delete({})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this possible now? What about thr superuser permissions issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we get permission error we set service as in-active and it won't show up on the front-end and the user can perform clean up using operate prune

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this command require sudo password? Can I confirm that the superuser permission only happens on Linux?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this required sudo. it does not happen on mac, we haven't tested on windows so cannot say

@truemiller
Copy link
Collaborator

small question before approval, are there are any deployment conflicts when redeploying a deleted agent that has only been set as inactive?

@angrybayblade
Copy link
Contributor Author

small question before approval, are there are any deployment conflicts when redeploying a deleted agent that has only been set as inactive?

Yes, the service in inactive status should not be deployed. Most probably it won't work but avoid requesting a deployment for an inactive service

@truemiller
Copy link
Collaborator

truemiller commented Feb 21, 2024

small question before approval, are there are any deployment conflicts when redeploying a deleted agent that has only been set as inactive?

Yes, the service in inactive status should not be deployed. Most probably it won't work but avoid requesting a deployment for an inactive service

Blocking the user from redeploying a deleted deployment is bad for UX. We should discuss with others to find a better solution.

Is it possible to chmod the directory to allow for deletion?
We could look into requesting a sudo/elevated privilege password via Electron for the python child_process.

@truemiller truemiller self-requested a review February 21, 2024 10:13
@angrybayblade angrybayblade merged commit 0655589 into feat/integration Feb 21, 2024
2 checks passed
@angrybayblade angrybayblade deleted the feat/delete-deployment branch April 11, 2024 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.

3 participants