Skip to content
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

Add statistics information for individual checks (especially time of execution) #1144

Closed
potiuk opened this issue Sep 22, 2019 · 13 comments · Fixed by #1257
Closed

Add statistics information for individual checks (especially time of execution) #1144

potiuk opened this issue Sep 22, 2019 · 13 comments · Fixed by #1257

Comments

@potiuk
Copy link

potiuk commented Sep 22, 2019

I would love to have an option to add some statistics to be displayed for each check. This could be very useful for running pre-commit in CI. I had a case recently of a check that run accidentally a bit longer than expected (it was building unnecessary docker image) and I have not realised that it is taking longer - because we had other checks after and I could only see the total execution time.

Also some other useful information that we might see is how many parallel processes were run and how many files were passed as parameters. This might be really useful especially for people who do not understand that pre-commit runs in parallel by default - this can have some undesireable side effects if you forget to set "require_serial" to True when you need. And seeing that in the output of CI might immediately show that something is wrong. For now the output is a bit of "black-box".

An option to see some additional information (--add-statistics) might be super-useful.

Example output I imagine (maybe a bit better aligned):

Check if image build is needed...........................................Passed [23.5s, 1 process, no-files]  
Add licence for all JINJA template files.................................Passed [2.25s, 8 processes, 48 files]
Check Shell scripts syntax correctness...................................Passed [6.00s, 8 processes, 10 files]
Lint dockerfile..........................................................Passed [2.20s, 3 processes, 3 files]
Run mypy.................................................................Passed [12.00s, 8 processes, 1456 files]
Run pylint for main sources.............................................Skipped 
Run pylint for tests....................................................Skipped
Run flake8...............................................................Passed [24.05s, 8 processes, 1456 files]
@potiuk
Copy link
Author

potiuk commented Sep 22, 2019

Happy to contribute it if there is an agreement to add it :)

@asottile
Copy link
Member

the output is intentionally quiet and I don't want to change that, your proposal also makes things no longer line up

maybe just add this to verbose if at all ?

@potiuk
Copy link
Author

potiuk commented Sep 22, 2019

verbose is .... far too verbose. It's great to see only "passed" but then having some stats, would make it also actionable for cases that are "valid properties" of the check rather than just passed/failed status. Most of the UnitTest frameworks have such feature to display for example the timing.

And it is perfectly ok to have a separate switch for that and do not have it in the output by default. I think it might be really useful for the CI case I described.

@asottile
Copy link
Member

asottile commented Sep 22, 2019

I don't want a combinatoric explosion of options, especially ones I'm not going to use

@asottile
Copy link
Member

if it's only for CI who cares how verbose it is?

@potiuk
Copy link
Author

potiuk commented Sep 22, 2019

I think it might be useful for local pre-commits as well, but the main usage scenario for this flag I foresee for CI case. We are currently using it with --all-files --show-diff-on-failure and it works great (minimum output + we see exact diffs when diff-failure happens).

In this case I only found out that we have a problem because the build steps were running for a really long time and failed (but I would immediately see the problem if I had the stats).
In my particular case we have the "build needed check" (see the #1143 ) and when it does not need to rebuild the image it will be "passed" in ms. But if (happened in my case) due to bug in build need detection it took maybe 2-3 minutes. - I would immediately see this with statistics but in case of our build the only thing i could see is "passed" without knowing how much time this particular check took.

BTW. I have found that having CI build for pre-commit is great promotor of pre-commit in general - because when something fails, installing pre-commit and running that failed check is simplest way to do (and pre-commit nicely suggests that action). But CI is also good to observe if other build properties are met.

@asottile
Copy link
Member

I don't want another option, let's add it to verbose output

@blueyed

This comment has been minimized.

@asottile
Copy link
Member

please read the discussion -- I've already said no to that

@blueyed

This comment has been minimized.

@pre-commit pre-commit deleted a comment from blueyed Nov 22, 2019
@asottile
Copy link
Member

please stay on topic, thanks

@blueyed

This comment has been minimized.

@asottile
Copy link
Member

asottile commented Jan 2, 2020

This has been released as part of v1.21.0 -- thanks again for the issue 🎉!

potiuk added a commit to PolideaInternal/airflow that referenced this issue Jan 18, 2020
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.
potiuk added a commit to apache/airflow that referenced this issue Jan 18, 2020
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.
potiuk added a commit to apache/airflow that referenced this issue Jan 21, 2020
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862)
kaxil pushed a commit to apache/airflow that referenced this issue Jan 22, 2020
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862)
kaxil pushed a commit to apache/airflow that referenced this issue Jan 23, 2020
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862)
potiuk added a commit to apache/airflow that referenced this issue Jan 26, 2020
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862)
kaxil pushed a commit to apache/airflow that referenced this issue Jan 26, 2020
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862)
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this issue Mar 5, 2020
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.
kaxil pushed a commit to astronomer/airflow that referenced this issue Mar 30, 2020
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862)
(cherry picked from commit 2969e50)
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Feb 11, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862efcbfcbbb939f08043b953216751ee22b)

GitOrigin-RevId: 2969e50b41337b4b7bf998d84c3f27385bb170e2
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Feb 22, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862efcbfcbbb939f08043b953216751ee22b)

GitOrigin-RevId: 2969e50b41337b4b7bf998d84c3f27385bb170e2
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Feb 23, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862efcbfcbbb939f08043b953216751ee22b)

GitOrigin-RevId: 2969e50b41337b4b7bf998d84c3f27385bb170e2
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Feb 24, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862efcbfcbbb939f08043b953216751ee22b)

GitOrigin-RevId: 2969e50b41337b4b7bf998d84c3f27385bb170e2
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jun 3, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862efcbfcbbb939f08043b953216751ee22b)

GitOrigin-RevId: 2969e50b41337b4b7bf998d84c3f27385bb170e2
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jun 4, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862efcbfcbbb939f08043b953216751ee22b)

GitOrigin-RevId: 2969e50b41337b4b7bf998d84c3f27385bb170e2
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jun 4, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862efcbfcbbb939f08043b953216751ee22b)

GitOrigin-RevId: 2969e50b41337b4b7bf998d84c3f27385bb170e2
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jun 7, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862efcbfcbbb939f08043b953216751ee22b)

GitOrigin-RevId: 2969e50b41337b4b7bf998d84c3f27385bb170e2
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jun 9, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

(cherry picked from commit c5e4862efcbfcbbb939f08043b953216751ee22b)

GitOrigin-RevId: 2969e50b41337b4b7bf998d84c3f27385bb170e2
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Sep 15, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Sep 17, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Sep 23, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Nov 25, 2021
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Mar 9, 2022
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jun 3, 2022
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jun 6, 2022
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
kosteev pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jul 8, 2022
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Aug 26, 2022
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Oct 3, 2022
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
aglipska pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Oct 7, 2022
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Dec 7, 2022
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jan 27, 2023
The latest version of pre-commit supports showing execution times
for particular checks. This was a feature requested in
pre-commit/pre-commit#1144 and they
finally implemented it after long time saying "no" :).

This commit enables it with --verbose flag - which is also useful
as it shows hook ids and some extra information printed by
some plugins.

GitOrigin-RevId: c5e4862efcbfcbbb939f08043b953216751ee22b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants