swarming: delete named cache symlinks before workdir

swarming: delete named cache symlinks before workdir

On Windows, shutil.rmtree is used to delete the workdir which removes
the actual named cache directories.

Unlink the symlinks before deleting the workdir

R=maruel@chromium.org, vadimsh@chromium.org
BUG=661390
Review-Url: https://codereview.chromium.org/2663023004
Committed: https://github.com/luci/luci-py/commit/a6efc1e4f6920dd1249ffb2bff3bf66ec6631e7d

[nodir, 2017-02-01 20:56:13]

PTAL

[M-A Ruel, 2017-02-01 20:58:27]

lgtm

https://codereview.chromium.org/2663023004/diff/20001/client/named_cache.py
File client/named_cache.py (right):

https://codereview.chromium.org/2663023004/diff/20001/client/named_cache.py#n...
client/named_cache.py:177: _validate_named_cache_path(path)
You think it's necessary?

[nodir, 2017-02-01 21:01:07]

https://codereview.chromium.org/2663023004/diff/20001/client/named_cache.py
File client/named_cache.py (right):

https://codereview.chromium.org/2663023004/diff/20001/client/named_cache.py#n...
client/named_cache.py:177: _validate_named_cache_path(path)
On 2017/02/01 20:58:27, M-A Ruel wrote:
> You think it's necessary?

the idea is that I don't want to delete files/dirs outside of the root

[M-A Ruel, 2017-02-02 01:11:10]

https://codereview.chromium.org/2663023004/diff/20001/client/named_cache.py
File client/named_cache.py (right):

https://codereview.chromium.org/2663023004/diff/20001/client/named_cache.py#n...
client/named_cache.py:177: _validate_named_cache_path(path)
On 2017/02/01 21:01:06, nodir wrote:
> On 2017/02/01 20:58:27, M-A Ruel wrote:
> > You think it's necessary?
> 
> the idea is that I don't want to delete files/dirs outside of the root

Oh sure but I would think they were validated in create_symlinks. It's fine.

[nodir, 2017-02-02 03:27:24]

On 2017/02/02 01:11:10, M-A Ruel wrote:
> https://codereview.chromium.org/2663023004/diff/20001/client/named_cache.py
> File client/named_cache.py (right):
> 
>
https://codereview.chromium.org/2663023004/diff/20001/client/named_cache.py#n...
> client/named_cache.py:177: _validate_named_cache_path(path)
> On 2017/02/01 21:01:06, nodir wrote:
> > On 2017/02/01 20:58:27, M-A Ruel wrote:
> > > You think it's necessary?
> > 
> > the idea is that I don't want to delete files/dirs outside of the root
> 
> Oh sure but I would think they were validated in create_symlinks. It's fine.

i don't want to assume that the parameters were validated before by calling
created_symlinks

[nodir, 2017-02-02 03:42:08]

The CQ bit was checked by nodir@chromium.org

[commit-bot: I haz the power, 2017-02-02 03:42:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-02 03:46:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-02 03:46:42]

Try jobs failed on following builders:
  Luci-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/3415bc01f6bfaa10)

[M-A Ruel, 2017-02-02 05:11:57]

https://codereview.chromium.org/2663023004/diff/20001/client/named_cache.py
File client/named_cache.py (right):

https://codereview.chromium.org/2663023004/diff/20001/client/named_cache.py#n...
client/named_cache.py:177: _validate_named_cache_path(path)
On 2017/02/02 01:11:10, M-A Ruel wrote:
> On 2017/02/01 21:01:06, nodir wrote:
> > On 2017/02/01 20:58:27, M-A Ruel wrote:
> > > You think it's necessary?
> > 
> > the idea is that I don't want to delete files/dirs outside of the root
> 
> Oh sure but I would think they were validated in create_symlinks. It's fine.

Sure, still lgtm

[nodir, 2017-02-02 17:13:20]

The CQ bit was checked by nodir@chromium.org

[nodir, 2017-02-02 17:13:20]

The patchset sent to the CQ was uploaded after l-g-t-m from maruel@chromium.org
Link to the patchset: https://codereview.chromium.org/2663023004/#ps40001
(title: "pylint")

[commit-bot: I haz the power, 2017-02-02 17:13:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-02 18:14:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-02 18:15:00]

Try jobs failed on following builders:
  Luci-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/3418a2aeaba07010)

[nodir, 2017-02-02 18:35:01]

The CQ bit was checked by nodir@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-02 18:35:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-02 18:37:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-02 18:37:35]

Dry run: This issue passed the CQ dry run.

[nodir, 2017-02-02 18:39:29]

The CQ bit was checked by nodir@chromium.org

[commit-bot: I haz the power, 2017-02-02 18:39:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-02 19:13:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-02 19:13:16]

Try jobs failed on following builders:
  Luci-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/34190dd669e22510)

[nodir, 2017-02-02 19:15:23]

The CQ bit was checked by nodir@chromium.org

[commit-bot: I haz the power, 2017-02-02 19:15:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nodir, 2017-02-02 19:34:42]

The CQ bit was unchecked by nodir@chromium.org

[nodir, 2017-02-02 19:34:46]

The CQ bit was checked by nodir@chromium.org

[commit-bot: I haz the power, 2017-02-02 19:34:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-02 20:16:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-02 20:16:58]

Try jobs failed on following builders:
  Luci-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/34192e6d914cc210)

[nodir, 2017-02-02 20:58:41]

The CQ bit was checked by nodir@chromium.org

[commit-bot: I haz the power, 2017-02-02 20:58:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-02 21:03:36]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486069121363000,
"parent_rev": "dacb7a1c8735cd76e362123e7183a716d9cdf99c", "commit_rev":
"a6efc1e4f6920dd1249ffb2bff3bf66ec6631e7d"}

[commit-bot: I haz the power, 2017-02-02 21:03:40]

Description was changed from

==========
swarming: delete named cache symlinks before workdir

On Windows, shutil.rmtree is used to delete the workdir which removes
the actual named cache directories.

Unlink the symlinks before deleting the workdir

R=maruel@chromium.org, vadimsh@chromium.org
BUG=661390
==========

to

==========
swarming: delete named cache symlinks before workdir

On Windows, shutil.rmtree is used to delete the workdir which removes
the actual named cache directories.

Unlink the symlinks before deleting the workdir

R=maruel@chromium.org, vadimsh@chromium.org
BUG=661390

Review-Url: https://codereview.chromium.org/2663023004
Committed:
https://github.com/luci/luci-py/commit/a6efc1e4f6920dd1249ffb2bff3bf66ec6631e7d
==========

[commit-bot: I haz the power, 2017-02-02 21:03:41]

Committed patchset #3 (id:40001) as
https://github.com/luci/luci-py/commit/a6efc1e4f6920dd1249ffb2bff3bf66ec6631e7d

[Vadim Sh., 2017-02-02 21:05:20]

On 2017/02/02 21:03:41, commit-bot: I haz the power wrote:
> Committed patchset #3 (id:40001) as
>
https://github.com/luci/luci-py/commit/a6efc1e4f6920dd1249ffb2bff3bf66ec6631e7d

https://goo.gl/SOAmEM