Preview Envs on Harvester: Delete when inactive#10379
Conversation
roboquat
added
do-not-merge/work-in-progress
do-not-merge/release-note-label-needed
size/M
size/L
and removed
size/M
labels
vulkoingim
force-pushed
the
aa/del-harvester
branch
from
212010d to
4f148e8
Compare
roboquat
added
release-note-none
and removed
do-not-merge/release-note-label-needed
labels
vulkoingim
changed the title
| Object.entries(result).forEach(([key]) => logLine+=` ${key}:${result[key]},`); | ||
| logLine = logLine.slice(0, -1); | ||
|
|
||
| werft.log(sliceID, `${previewEnvironment.name} (${previewEnvironment.namespace}) - is-active=${isActive} ${logLine}`) |
There was a problem hiding this comment.
Not a fan of logging here, but it does make things cleaner
vulkoingim
force-pushed
the
aa/del-harvester
branch
2 times, most recently
from
6327cb2 to
db7a143
Compare
vulkoingim
force-pushed
the
aa/del-harvester
branch
from
8dc2474 to
1d0d1d9
Compare
mads-hartmann
left a comment
mads-hartmann
left a comment
There was a problem hiding this comment.
Thanks for working on this, and I agree that removing the double negative is a nice improvement. I have left a few suggestions for improvements, but they can be done in a follow up PR if you prefer (I have added the hold label, so you can just remove it if you want to merge this and open a new PR).
This does not solve all of #9891 yet though, as currently we'd still delete the preview environment if e.g. the branch was missing or stale (it was mentioned as a comment on the issue). For that you'd need to modify the logic for previewsToDelete
vulkoingim
force-pushed
the
aa/del-harvester
branch
3 times, most recently
from
213815b to
4c6227a
Compare
mads-hartmann
left a comment
mads-hartmann
left a comment
There was a problem hiding this comment.
One tiny boolean that I think you have to flip, but otherwise, LGTM
vulkoingim
force-pushed
the
aa/del-harvester
branch
from
4c6227a to
86864d9
Compare
3 participants
Description
is-inactive->is-active, as double negatives make for a confusing experience (CHECKING_FOR_NO_DB_ACTIVITY, isInactive = false = actually active env, etc)Related Issue(s)
Fixes #9891
How to test
Following is a
platform-delete-preview-environmentsjob (no preview environments were harmed, as all are returned as active on that run)https://werft.gitpod-dev.com/job/gitpod-custom-aa-del-harvester.28
Release Notes