-
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
Support for deleting deployments #15
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,8 +144,9 @@ async def access( | |
def json(self) -> GetServices: | ||
"""Returns the list of available services.""" | ||
data = [] | ||
for service in self.path.iterdir(): | ||
data.append(Service.load(path=service).json) | ||
for path in self.path.iterdir(): | ||
service = Service.load(path=path) | ||
data.append(service.json) | ||
return data | ||
|
||
def _stake(self) -> None: | ||
|
@@ -337,12 +338,7 @@ def update(self, data: PutServices) -> ServiceType: | |
reuse_multisig=True, | ||
update_token=old.chain_data["token"], | ||
) | ||
|
||
# try: | ||
# shutil.rmtree(old.path) | ||
# except Exception as e: | ||
# print(e) | ||
|
||
old.delete({}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this possible now? What about thr superuser permissions issue? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
return service.json | ||
|
||
def delete(self, data: DeleteServicesPayload) -> ServicesType: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,9 @@ description = "" | |
authors = ["David Vilela <[email protected]>"] | ||
readme = "README.md" | ||
|
||
[tool.poetry.scripts] | ||
operate = "operate.cli:main" | ||
|
||
[tool.poetry.dependencies] | ||
python = "^3.10" | ||
open-autonomy = "^0.14.2" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,8 +27,9 @@ export type Service = { | |
hash: string; | ||
keys: Keys; | ||
readme?: string; | ||
ledger?: LedgerConfig; | ||
chain_data?: ChainData; | ||
ledger: LedgerConfig; | ||
chain_data: ChainData; | ||
active: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
}; | ||
|
||
export type ServiceTemplate = { | ||
|
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.
@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 deploymentsThere 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.
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?