Pull request for #85: Find way to speed up log.py by arosenbe · Pull Request #87 · gslab-econ/gslab_python

arosenbe · 2017-07-26T22:53:16Z

@stanfordquan can you please PR my work from #85? I replaced os.walk with a (platform-specific) subprocess calling a command line tool used to search for files with a certain name. Let me know if you have any questions!

qlquanle

Good job @arosenbe! Some formatting comment. The script works well as far as I can tell. Thanks!

qlquanle · 2017-07-27T03:43:10Z

+    rel_parent_dir = os.path.relpath(parent_dir)
+    log_name = 'sconscript*.log'
+    if misc.is_unix():
+        cmd = 'find %s -name "%s"' % (rel_parent_dir, log_name)


different variable name please? this is like naming something bash.

does this catch the wildcard as intended? (i.e. sconscript_@!#ASD.log?)

Now called command, thanks for catching that silly name.

It does catch that name on Mac.

mkdir temp # Watch paths mkdir temp/temp2 touch "temp/temp2/sconscript_@\!\#ASD.log" find . -name "sconscript*.log" rm -rf temp # BEWARE

qlquanle · 2017-07-27T03:44:24Z

-                        builder_log_end_time = beginning_of_time
-                builder_log_collect[f_full]  = builder_log_end_time
+    # Store paths to logs in a list, found from platform-specific command line tool 
+    rel_parent_dir = os.path.relpath(parent_dir)


qlquanle · 2017-07-27T04:49:16Z

+        cmd = 'dir "%s" /b/s' % os.path.join(rel_parent_dir, log_name)
+    try:
+        log_paths = subprocess.check_output(cmd, shell = True).replace('\r\n', '\n')
+        log_paths = log_paths.split('\n')[:-1] # Last entry always ''


please assert this before subsetting (perhaps with an if statement)

Now uses filter and map to avoid hard coding anything.

qlquanle · 2017-07-27T04:53:09Z

@@ -52,7 +53,7 @@ def end_log(log = 'sconstruct.log'):
            with open(f, 'rU') as sconscript:
                if this_run_dict[f] == beginning_of_time:
                    sconstruct.write("*** Warning!!! Doesn't look the sconscript below finished.\n")


Can you please do me a favor and change this one to " Warning!!! The log below does not have timestamps, the Sconscript may not have finished."

Sure thing. I assumed you wanted to keep the new line at the end, but let me know if not.

…w warning msg

arosenbe · 2017-07-27T16:43:40Z

Thanks for the good suggestions @stanfordquan! I think 0c67612 above takes care of them, but let me know if not.

* Pull request for #28: gslab_scons restructure (#47) * #28 modify check_code_extension() * #28 make distinction between `target_dir` and `log_dir` for all but `build_stata()`, add code flow comments in builder * #28 minor change, `return None` switched to `return False` for logical check * #28 add informative BadExecutableError messages * #28 merging remote to local * #28 add single log file + appending mechanism * #28 refactor build_stata() * #28 add formatting for logs * #28 fix typo in docstring * #28 update error messages and tests * #28 add mode check in start_log() * #28 change formatting in master tests to match work in this task * #47 dev commit to work from home * #47 rework logging different appending mechanism from builder logs to sconstruct.log * #28 respond to PR * #28 make pretty * #28 fix lyx pdf issue * #28 respond to more comments * #28 fix typo * Pull request for #59: Fix size warnings (#60) * #59: Redesigned size testing * #59: debugging * #59: additional work on list_ignored_files() * #59: Additional testing progress * #59: Brought tests to 100% coverage * change * #60: responded to PR * 60: response to PR * #59: brought size_warning tests to 100% coverage * #59: removed sys.exit() and raw_input() from size warning function * #59: scons debrief function * #59: altered size warnings * #59: small revisions to size warnings * #60: misc changes * Add note about sudo * Update README.md * Pull request for #43 in Template: Configuration tests (#66) * Migrates configuration tests * Preliminary tests setup * Additional tests * More edits * Adds more tests * Adds more tests * Adds more tests * Adds more tests * Adds more unit tests * Adds more unit tests * Fixes load_yaml_value tests * Dev commit * Dev commit * Brings coverage to 100 percent * Changes name * Edits tests for dropping --sf command line call * Changes name back * Fixes name in function * #66 Implements prelim PR comments * #66 Other PR comments * #66: Standardised exception classes * #66: updated some tests * #66: misc changes to tests * #66: some more changes * #66: changed error message * #66: removed extra newline from yaml creation * #66: aesthetic changes * #66: updated version number to 4 * Pull request for gslab-econ/template#45: Standardise stata_executable (#67) * gslab-econ/template#45 Changed to 'stata_executable' * Fixed a comment... * #67: Responses to PR * #67: Response to PR * Pull request for #58: Prepare for 4.0 (#65) * #58 cosmetic changes * Issue 58 - implement comments to builders 1) Fix R builder command line argument issue. Will test if issue persists for other builders. 2) Time-log failure is now kinder (does not block completion of SConstruct, just raise a noisy warning) * #58 small changes during merge * Pull request for #69: Update tablefill builder with 4.0 logging machinery (#70) * #69 add logging functionalities * #69 decrease clutter in sconscript.log * #69 prettify output * Pull request for #75: Print traceback for python builder (#76) * #75 cat \nTraceback to error message in Python builder * #75 remove new line * #75 fix crashing log.py if sconscript.log is empty * Pull request for #68: Remove Google Drive from default release (#72) * issue #68 - Fix broken imports in test - Move `read_yaml_value` to `misc` from `config` - Fix appropriate tests broken because of above move * #68 accommodate atexit implementation of state_of_repo * #68 fix numargs * #68 - instead of _ for user-config.yaml * #68 fix regex error in _release_tools * #68 respond to PR * #68 move check_and_expand_cache_path to misc.py, rename check_and_expand_path * #68 response to PR about release paths * #68 remove prompt for github token * #68 respond to PR * #68 add logical gate to use getpass.getpass for github_token * Pull request for #78: Updates readme (#79) * #78 Updates readme * Update README.md * Pull request for #73: Allows tablefill to use tex documents (#82) * #73 Preliminary implementation * #73 Updates info * #73 add newline after log for easy viewing * Pull request for #77: Replace <*> with <source file name> in <sconscript_*.log> (#80) * #77 add optional log extension through way of log_ext in ENV * #77 revert matlab reordering * #77 fix GitKraken merge typos * 77 add newline back in tablefill logging * #77 fix unittests * #77 respond to PR * #77 remove stray backslash * #77 fix broken unittests that allow dumb input as env * #83 add coverage to dependencies (#84) * Pull request for #85: Find way to speed up log.py (#87) * #85 log search with CL tool * #85 new log search works for mac * #85 get logging working on windows * #87 #85 var named cmd -> command, filter + map removes null paths, new warning msg * Update log.py * Update README.md * Update README.md * Update release.py * #88 format release instructions * #88 remove repeated word in readme

arosenbe and others added 3 commits July 26, 2017 09:22

#85 log search with CL tool

7a59a25

#85 new log search works for mac

7841d08

#85 get logging working on windows

e4c86d6

arosenbe requested a review from qlquanle July 26, 2017 22:53

arosenbe mentioned this pull request Jul 26, 2017

Find way to speed up log.py #85

Closed

arosenbe self-assigned this Jul 26, 2017

qlquanle suggested changes Jul 27, 2017

View reviewed changes

qlquanle reviewed Jul 27, 2017

View reviewed changes

#87 #85 var named cmd -> command, filter + map removes null paths, ne…

0c67612

…w warning msg

Update log.py

bb6855f

qlquanle approved these changes Jul 27, 2017

View reviewed changes

qlquanle merged commit e44453c into dev_four Jul 27, 2017

qlquanle deleted the issue85-fastlog branch July 27, 2017 19:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pull request for #85: Find way to speed up log.py#87

Pull request for #85: Find way to speed up log.py#87
qlquanle merged 5 commits into
dev_fourfrom
issue85-fastlog

arosenbe commented Jul 26, 2017

Uh oh!

qlquanle left a comment

Uh oh!

qlquanle Jul 27, 2017

Uh oh!

qlquanle Jul 27, 2017

Uh oh!

arosenbe Jul 27, 2017

Uh oh!

arosenbe Jul 27, 2017

Uh oh!

qlquanle Jul 27, 2017

Uh oh!

qlquanle Jul 27, 2017

Uh oh!

arosenbe Jul 27, 2017 •

edited

Loading

Uh oh!

qlquanle Jul 27, 2017

Uh oh!

arosenbe Jul 27, 2017

Uh oh!

arosenbe commented Jul 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

arosenbe commented Jul 26, 2017

Uh oh!

qlquanle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arosenbe Jul 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arosenbe commented Jul 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arosenbe Jul 27, 2017 •

edited

Loading