Add file.filesizes to recipe engine core modules.

Add file.filesizes to recipe engine core modules.

R=iannucci@chromium.org, machenbach@chromium.org
BUG=v8:6673
Review-Url: https://codereview.chromium.org/2993713003
Committed: https://github.com/luci/recipes-py/commit/3cae8e948be4bd6a43a65e121b2e108111882396

[tandrii(chromium), 2017-08-04 16:34:38]

it was a fun exercise to write this. Robbie thanks for normalizing docs in
recipes-py.

[Michael Achenbach, 2017-08-07 07:37:40]

lgtm with suggestions.

Maybe also refer to bug v8:6673 where this will be used.

https://codereview.chromium.org/2993713003/diff/1/recipe_modules/file/api.py
File recipe_modules/file/api.py (right):

https://codereview.chromium.org/2993713003/diff/1/recipe_modules/file/api.py#...
recipe_modules/file/api.py:228: """Returns list of filesizes for a given files.
s/a/the

https://codereview.chromium.org/2993713003/diff/1/recipe_modules/file/api.py#...
recipe_modules/file/api.py:236: test_data = kwargs.pop('test_data', ())
How about some better fake data when no test data is provided? E.g. some fake
sizes that fit to the number of files?

https://codereview.chromium.org/2993713003/diff/1/recipe_modules/file/resourc...
File recipe_modules/file/resources/fileutil.py (right):

https://codereview.chromium.org/2993713003/diff/1/recipe_modules/file/resourc...
recipe_modules/file/resources/fileutil.py:259: for f in opts.file)))
nit: The for loop should be indented on the level of "str(os.stat(f).st_size)"

[tandrii(chromium), 2017-08-07 10:36:00]

Description was changed from

==========
Add file.filesizes to recipe engine core modules.

R=iannucci@chromium.org, machenbach@chromium.org
BUG=
==========

to

==========
Add file.filesizes to recipe engine core modules.

R=iannucci@chromium.org, machenbach@chromium.org
BUG=v8:6673
==========

[tandrii(chromium), 2017-08-07 10:59:00]

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

[commit-bot: I haz the power, 2017-08-07 10:59:09]

Dry run: CQ is trying da patch.

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

[tandrii(chromium), 2017-08-07 10:59:49]

https://codereview.chromium.org/2993713003/diff/1/recipe_modules/file/api.py
File recipe_modules/file/api.py (right):

https://codereview.chromium.org/2993713003/diff/1/recipe_modules/file/api.py#...
recipe_modules/file/api.py:228: """Returns list of filesizes for a given files.
On 2017/08/07 07:37:40, Michael Achenbach wrote:
> s/a/the

Done.

https://codereview.chromium.org/2993713003/diff/1/recipe_modules/file/api.py#...
recipe_modules/file/api.py:236: test_data = kwargs.pop('test_data', ())
On 2017/08/07 07:37:39, Michael Achenbach wrote:
> How about some better fake data when no test data is provided? E.g. some fake
> sizes that fit to the number of files?

Done.

https://codereview.chromium.org/2993713003/diff/1/recipe_modules/file/resourc...
File recipe_modules/file/resources/fileutil.py (right):

https://codereview.chromium.org/2993713003/diff/1/recipe_modules/file/resourc...
recipe_modules/file/resources/fileutil.py:259: for f in opts.file)))
On 2017/08/07 07:37:40, Michael Achenbach wrote:
> nit: The for loop should be indented on the level of "str(os.stat(f).st_size)"

Done.

[Michael Achenbach, 2017-08-07 11:05:57]

lgtm

[commit-bot: I haz the power, 2017-08-07 11:06:19]

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

[commit-bot: I haz the power, 2017-08-07 11:06:20]

Dry run: Try jobs failed on following builders:
  Recipes-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/37d52ae93da91f10)

[tandrii(chromium), 2017-08-07 11:55:39]

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

[tandrii(chromium), 2017-08-07 11:55:46]

Robbie, PTAL

[commit-bot: I haz the power, 2017-08-07 11:55:49]

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-08-07 12:02:19]

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

[commit-bot: I haz the power, 2017-08-07 12:02:20]

Dry run: Try jobs failed on following builders:
  Recipes-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/37d562204fe1dd10)

[tandrii(chromium), 2017-08-07 12:03:05]

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

[commit-bot: I haz the power, 2017-08-07 12:03:11]

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-08-07 12:09:00]

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

[commit-bot: I haz the power, 2017-08-07 12:09:01]

Dry run: This issue passed the CQ dry run.

[iannucci, 2017-08-07 19:27:30]

lgtm % kwargs

https://codereview.chromium.org/2993713003/diff/60001/recipe_modules/file/api.py
File recipe_modules/file/api.py (right):

https://codereview.chromium.org/2993713003/diff/60001/recipe_modules/file/api...
recipe_modules/file/api.py:227: def filesizes(self, name, files, **kwargs):
No kwargs; we're trying to stamp this out everywhere. Look at the other methods
here to see how to do test data.

[tandrii(chromium), 2017-08-07 19:31:11]

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

[commit-bot: I haz the power, 2017-08-07 19:31:25]

Dry run: CQ is trying da patch.

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

[tandrii(chromium), 2017-08-07 19:32:39]

On 2017/08/07 19:27:30, iannucci wrote:
> lgtm % kwargs
> 
>
https://codereview.chromium.org/2993713003/diff/60001/recipe_modules/file/api.py
> File recipe_modules/file/api.py (right):
> 
>
https://codereview.chromium.org/2993713003/diff/60001/recipe_modules/file/api...
> recipe_modules/file/api.py:227: def filesizes(self, name, files, **kwargs):
> No kwargs; we're trying to stamp this out everywhere. 

I missed this. Because of what? Leaky abstraction?

> Look at the other methods
> here to see how to do test data.
Done.

[commit-bot: I haz the power, 2017-08-07 19:37:31]

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

[commit-bot: I haz the power, 2017-08-07 19:37:32]

Dry run: This issue passed the CQ dry run.

[tandrii(chromium), 2017-08-08 11:41:57]

The CQ bit was checked by tandrii@chromium.org

[tandrii(chromium), 2017-08-08 11:41:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
machenbach@chromium.org, iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/2993713003/#ps80001
(title: "-kwargs.")

[commit-bot: I haz the power, 2017-08-08 11:42:08]

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-08-08 11:45:39]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1502192517975500,
"parent_rev": "862f33385b0c24846ea7850b4c1bdf646cafcf60", "commit_rev":
"3cae8e948be4bd6a43a65e121b2e108111882396"}

[commit-bot: I haz the power, 2017-08-08 11:45:44]

Description was changed from

==========
Add file.filesizes to recipe engine core modules.

R=iannucci@chromium.org, machenbach@chromium.org
BUG=v8:6673
==========

to

==========
Add file.filesizes to recipe engine core modules.

R=iannucci@chromium.org, machenbach@chromium.org
BUG=v8:6673

Review-Url: https://codereview.chromium.org/2993713003
Committed:
https://github.com/luci/recipes-py/commit/3cae8e948be4bd6a43a65e121b2e1081118...
==========

[commit-bot: I haz the power, 2017-08-08 11:45:45]

Committed patchset #5 (id:80001) as
https://github.com/luci/recipes-py/commit/3cae8e948be4bd6a43a65e121b2e1081118...