-
Notifications
You must be signed in to change notification settings - Fork 52
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
Update build metadata on deletion, fix misc bugs #572
Update build metadata on deletion, fix misc bugs #572
Conversation
✅ Deploy Preview for kaleidoscopic-dango-0cf31d canceled.
|
@@ -231,7 +232,7 @@ def delete_build_artifact(db: Session, conda_store, build_artifact): | |||
# ignore key | |||
conda_prefix = build_artifact.build.build_path(conda_store) | |||
# be REALLY sure this is a directory within store directory | |||
if conda_prefix.startswith(conda_store.store_directory) and os.path.isdir( | |||
if str(conda_prefix).startswith(conda_store.store_directory) and os.path.isdir( |
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.
Bug fix: PosixPath
object (conda_prefix
) has no attribute startswith
, so need to convert to str
first
build.deleted_on = datetime.datetime.utcnow() | ||
build.size = 0 | ||
|
||
db.commit() |
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.
Bug fix: CondaStore
object has no attribute db
, need to just use db
for this
|
||
# Updates build size and marks build as deleted | ||
build.deleted_on = datetime.datetime.utcnow() | ||
build.size = 0 |
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.
Metadata fix: size value of the current build is shown in the web UI as size
. Need to update that when deleting artifacts here.
How it works in more detail:
{% for environment in environments %}
<div class="card my-2">
<div class="card-body">
<h5 class="card-title">
<a href="{{ url_for('ui_get_environment', namespace=environment.namespace.name, environment_name=environment.name) }}">{{ environment.namespace.name }} / <span class="badge badge-info">{{ environment.name }}</span></a>
<span class="badge badge-light">{{ (environment.current_build.size or 0) | filesizeformat(true) }}</span>
</h5>
=>
document.querySelectorAll('button[data-action="delete"]').forEach(item => {
item.addEventListener('click', (event) => deleteBuild(event));
})
=>
async function deleteBuild(event) {
...
let url = `{{ url_for('api_list_builds') }}${ buildId }/`;
let response = await fetch(url, { method: 'DELETE' });
...
=>
@router_api.delete(
"/build/{build_id}/",
response_model=schema.APIAckResponse,
)
async def api_delete_build(
...
conda_store.delete_build(db, build_id)
=>
def delete_build(self, db: Session, build_id: int):
...
tasks.task_delete_build.si(build.id).apply_async()
=>
task_delete_build (this function)
To test that this works:
|
Tested locally, and code reviewed : It works, with PR 574 being merged. So merge 574 before merging this one. |
Fixes #560.