Add timestamp to scan summary #925 #945

techytushar · 2018-02-18T21:16:22Z

PR for issue #925
Signed-off-by: Tushar Mittal [email protected]

codecov · 2018-02-18T21:59:01Z

Codecov Report

Merging #945 into develop will decrease coverage by 2.01%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop     #945      +/-   ##
===========================================
- Coverage     78.9%   76.88%   -2.02%     
===========================================
  Files          103      105       +2     
  Lines        12670    13984    +1314     
===========================================
+ Hits          9997    10752     +755     
- Misses        2673     3232     +559

Impacted Files	Coverage Δ
src/formattedcode/output_json.py	`100% <ø> (ø)`	⬆️
src/formattedcode/output_jsonlines.py	`100% <ø> (ø)`	⬆️
src/scancode/cli_test_utils.py	`76.81% <100%> (-3.19%)`	⬇️
src/scancode/resource.py	`73.95% <100%> (-7.87%)`	⬇️
src/scancode/cli.py	`77.68% <100%> (+0.33%)`	⬆️
src/cluecode/copyrights.py	`65.1% <0%> (-32.2%)`	⬇️
src/licensedcode/frequent_tokens.py	`77.77% <0%> (-22.23%)`	⬇️
src/scancode/api.py	`71.2% <0%> (-21.6%)`	⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7884468...c93f823. Read the comment docs.

pombredanne

Thanks!
OPne thing beside the comments is that we need to have the scan start timestamp added to the scan headers outputs, at least in the JSON ones

pombredanne · 2018-02-19T12:45:19Z

src/scancode/cli.py

@@ -47,7 +47,7 @@
 from scancode_config import scancode_temp_dir

 from commoncode.fileutils import PATH_TYPE
-from commoncode.timeutils import time2tstamp
+from commoncode.timeutils import time2tstamp,tstamp2time


Please keep each import on a separate line, but also based on my other comments tstamp2time is not likely needed at all.

IMHO time2tstamp returns date time like this 2018-02-19T145640.979020 which is quite hard to read and understand so i converted it to 2018-02-19 14:56:40.979020 using tstamp2time

IMHO It is pure luck if a datetime ends up being serialized alright.
Instead you should use this I just pushed 179d983 with path_safe=False

pombredanne · 2018-02-19T12:48:02Z

src/scancode/cli.py

@@ -1202,7 +1202,14 @@ def display_summary(codebase, scan_names, processes, verbose):
                                '%(final_size_count)s' % locals())

    echo_stderr('Timings:')
+
+    timestamp = codebase.timings.get('timestamp',None)
+    timestamp = tstamp2time(timestamp)


Why do you get a time back?

pombredanne · 2018-02-19T12:49:02Z

src/scancode/cli.py

    for name, value, in codebase.timings.items():
+        if name == 'timestamp':


You should pop() above instead IMHO
e.g. not timestamp = codebase.timings.get('timestamp',None) but instead timestamp = codebase.timings.pop('timestamp',None)

But this should not be in timings... but a proper attribute, so this is moot.

pombredanne · 2018-02-19T12:50:37Z

src/scancode/cli.py

@@ -724,7 +724,7 @@ def scancode(ctx, input,  # NOQA
        # TODO: this is weird: may be the timings should NOt be stored on the
        # codebase, since they exist in abstract of it??
        codebase.timings.update(setup_timings)
-
+        codebase.timings['timestamp'] = scan_start


IMHO the scan_start should be its own attribute on the Codebase. This is not a "timing" proper.

@techytushar

* this is based on discussions with @techytushar in #945 Signed-off-by: Philippe Ombredanne <[email protected]>

techytushar · 2018-02-19T15:33:53Z

@pombredanne please review, i have made the required changes.. 😅

pombredanne

Thanks. This now needs some tests for sure. Testing the presence of this timestamp (and not its value since it changes on each run)

pombredanne · 2018-02-19T15:47:56Z

src/formattedcode/output_json.py

                   pretty_options=pretty_options,
                   pretty=True)


 def write_json(results, output_file, files_count,
               scancode_version, scancode_notice,
-               pretty_options, pretty=False):
+               timestamp,pretty_options, 


your code is not properly formatted

techytushar · 2018-02-21T00:03:44Z

@pombredanne please review, added the test

pombredanne · 2018-02-27T00:37:27Z

src/scancode/cli.py

@@ -1202,6 +1202,10 @@ def display_summary(codebase, scan_names, processes, verbose):
                                '%(final_size_count)s' % locals())

    echo_stderr('Timings:')
+
+    timestamp = codebase.scan_start
+    echo_stderr('  timestamp: %(timestamp)s'% locals())


Could we call this Scan start instead here and in the JSON output? ... since this is what this is.

Signed-off-by: Tushar Mittal <[email protected]>

techytushar · 2018-02-28T09:34:38Z

@pombredanne only one test is failing in the travis-ci, but all tests pass on my local machine, please review..

pombredanne · 2018-03-23T03:46:33Z

The travis failure is an heisenbug. I respun a build

pombredanne · 2018-05-25T21:46:45Z

Merging now. Thanks!

pombredanne requested changes Feb 19, 2018

View reviewed changes

techytushar force-pushed the timestamp branch from ed98c29 to 66f428b Compare February 19, 2018 15:26

pombredanne added a commit that referenced this pull request Feb 19, 2018

Add option to create proper ISO timestamps #925

179d983

* this is based on discussions with @techytushar in #945 Signed-off-by: Philippe Ombredanne <[email protected]>

pombredanne requested changes Feb 19, 2018

View reviewed changes

techytushar force-pushed the timestamp branch 2 times, most recently from 4dfec29 to 8c904f3 Compare February 20, 2018 23:59

techytushar force-pushed the timestamp branch from 8c904f3 to 3d4010b Compare February 21, 2018 00:44

pombredanne reviewed Feb 27, 2018

View reviewed changes

techytushar force-pushed the timestamp branch from 3d4010b to da96172 Compare February 28, 2018 05:42

Add timestamp to scan summary and json output file aboutcode-org#925

c93f823

Signed-off-by: Tushar Mittal <[email protected]>

techytushar force-pushed the timestamp branch from da96172 to c93f823 Compare February 28, 2018 08:04

pombredanne merged commit ec96739 into aboutcode-org:develop May 25, 2018

pombredanne mentioned this pull request Feb 2, 2022

Create common header for ABC Data. #2841

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add timestamp to scan summary #925 #945

Add timestamp to scan summary #925 #945

techytushar commented Feb 18, 2018

codecov bot commented Feb 18, 2018 •

edited

Loading

pombredanne left a comment

pombredanne Feb 19, 2018

techytushar Feb 19, 2018

pombredanne Feb 19, 2018

pombredanne Feb 19, 2018

pombredanne Feb 19, 2018

pombredanne Feb 19, 2018

pombredanne Feb 19, 2018

techytushar commented Feb 19, 2018

pombredanne left a comment

pombredanne Feb 19, 2018

techytushar commented Feb 21, 2018 •

edited

Loading

pombredanne Feb 27, 2018

techytushar commented Feb 28, 2018 •

edited

Loading

pombredanne commented Mar 23, 2018

pombredanne commented May 25, 2018

		for name, value, in codebase.timings.items():
		if name == 'timestamp':

Add timestamp to scan summary #925 #945

Add timestamp to scan summary #925 #945

Conversation

techytushar commented Feb 18, 2018

codecov bot commented Feb 18, 2018 • edited Loading

Codecov Report

pombredanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techytushar commented Feb 19, 2018

pombredanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techytushar commented Feb 21, 2018 • edited Loading

Choose a reason for hiding this comment

techytushar commented Feb 28, 2018 • edited Loading

pombredanne commented Mar 23, 2018

pombredanne commented May 25, 2018

codecov bot commented Feb 18, 2018 •

edited

Loading

techytushar commented Feb 21, 2018 •

edited

Loading

techytushar commented Feb 28, 2018 •

edited

Loading