Pull request for #28: gslab_scons restructure by qlquanle · Pull Request #47 · gslab-econ/gslab_python

qlquanle · 2017-01-19T19:11:53Z

Changelog:

<SConscript>
target = '#output/plot.eps'
source = '#source/analysis.do'
assets = '#source/raw/data.txt'
env.Stata(target, source, assets)

where the builder definition in SConstruct is modified so that it runs

env.Depends(target, assets)
build_stata(target, source)

but this requires frontloading the Builders definitions with more scripting that isn't necessarily cleaner than my proposed solution here. Either way, there's no gslab_python code change, just convention changes in template. I could have restrict "source" to be single-element string but that would be backward incompatible which I think is unnecessary at this point for our lab and collaborators.

restructuring log file
Log files are now copied and appended into the overall SConstruct log in the following order:

start time
builder log
builder log
Sconstruct log

SConstruct log also no longer overwrite with each build but rather append. I do not yet delete the SConscript.log files, which have the same behaviour as before: overwrite with each build. I figure it'll make it easier to debug specific logs, but I'm happy to simply delete the SConscript.log file after build.
I have some concerns about this implementation, in particular: accumulating records may introduce significant merge conflicts. Branches A and B from Master will have different log files after merging in. Even with timestamps, it'll make things confusing. I think that Git history tool or blame tool are appropriate for this use of searching the most recent run of a particular build, or printing out SConsign file, rather than this implementation.

#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

Conflicts: gslab_scons/tests/log/make.log

arosenbe · 2017-01-19T21:23:09Z

Request noted. Barring something new on congress_text, this is up next for me.

arosenbe

Nice job @stanfordquan! Don't let the number of comments suggest that your work wasn't very good. They're more my desire to make sure that everything is up to snuff.

To your specific comments in the changelog. Do you want my comments on the decisions you made, or are you just reporting them?

Some comments of my own that I couldn't fit into particular comments on the code.

It might be good to document in a README the way in which we specify executables in the source and target lists in SCons (i.e. the first element of the list.) This would remove redundancy in the docstrings of the individual functions.
- We could also use the README to give instructions on how to use the builders.
There's a typo in the docstring to the current_time function in misc.py.
Update the version in the README
As a verb, one should use "set up."
It seems like there's some redundancy in the Prelims, Close log, and Append builder-log, etc. sections across the builders. Can I get your opinion on handling them in a single more unified function? For the record, I do like how clean and clear the current system is, and I'm not positive that abstraction would improve readability (the tradeoff between reading something once/making explicit that different applications use the same logic on the one hand and the benefit of keeping everything more or less in the same place on the other). This is a point for discussion, not a recommendation.
- I think we should be using 'ab' and 'rU' when writing to and reading from logs.
- We're vulnerable to race conditions because several scripts may be reading from and writing to the same logs at the same time. To the extent that the order of output in the logs is useful to us, we have to modify this.
- How should we handle logging when one script builds targets across several directories in build?
- How do you want to handle the concerns you note in the restructuring log file box.
I prefer to keep import statements from third_party and GSLab modules together. See the from sys... import in build_stata.
build_stata.py is missing the # headings to break the script into logical chunks.
I'm not sure how I feel about the stata helpers living in the same place as the more general helpers. It might create headache for future users if they have to scroll through several function with possibly relevant names in order to find the function their looking for.
The make_list_if_string function will also make lists out of lists, dictionaries, ints, pandas data frames, etc.
There are different numbers of blank lines before the return statement in each builder.

arosenbe · 2017-01-20T18:01:40Z

    start_time  = misc.current_time()
+
+    # Setup source file
    source_file = str(source[0])


What are the cases when the first element of the "source list" would not be already be a string? If there are such cases, do we want to coerce to a string and try to run, or do we want to raise an Exception, or do we want to leave "as is" and have SCons raise an exception when it tries to execute, say, an int? The last might be the most Pythonic, and I'd be in favor of the first. This applies to all builders.

arosenbe · 2017-01-20T18:06:14Z

-def check_code_extension(source_file, software):
+def check_code_extension(source_file, extension):
    '''
    This function raises an exception if `source_file`'s extension


I'm not a huge fan of the visual on

`source_file`'s

Maybe we could change it to the extension of source_file? What do you think?

arosenbe · 2017-01-20T18:08:15Z

+    misc.check_code_extension(source_file, '.lyx')
+
+    # Setup target file and log file
+    newpdf      = source_file.replace('.lyx','.pdf')


This won't handle edge.lyx.case.lyx well.

arosenbe · 2017-01-20T18:33:24Z

+        # Move rendered pdf to the target
+        shutil.move(newpdf, target_file)
+    except subprocess.CalledProcessError:
+        message = system_call_error("lyx", command)


I can't seem to find the system_call_error function. Did you mean command_error_msg? Applies to all such cases.

arosenbe · 2017-01-20T18:34:27Z

 import os
+import subprocess
 import shutil
 import gslab_scons.misc as misc


Do we want to specify the particular functions that we import from gslab_scons.misc? I'm not sure, but it seems silly to always drag around the Stata stuff into a Lyx builder. If so, then this applies to all such import statements.

I'm agnostic about this, so I'll implement it.

On second thought, I disagree. There are only three Stata only helper functions in misc; this method means we'll be importing four to five helper functions separately and I'm not convinced that will be cleaner than the current implementation.

arosenbe · 2017-01-20T23:54:40Z

@@ -2,27 +2,42 @@
 import sys


The import statements in this file are structured differently than those in the builders. Could we treat them in a unified manner?

arosenbe · 2017-01-20T23:56:34Z

+
+    if not (mode in ['develop', 'cache', 'release']):
+        print("Error: %s is not a defined mode" % mode)
+        sys.exit()


I assume there'a a reason, but can you explain why we prefer the print and sys.exit combination to the more parsimonious raise Exception('message') option?

Not really, changed.

arosenbe · 2017-01-20T23:57:42Z

+        print("Error: Version must be defined in release mode")
+        sys.exit()
+
+    start_message = "\n{0}{0}{0} New build: " + misc.current_time() + " {0}{0}{0}"


This might be better handled with sprintf style syntax.

good with me

arosenbe · 2017-01-20T23:58:39Z

        f.seek(0, 0)
-        f.write('Log created:    ' + start_time + '\n' + 
-                'Log completed:  ' + end_time   + '\n \n' + content)
+        f.write('\n*** Builder log created:    ' + start_time + '\n' + 


Maybe sprintf format here as well?

arosenbe · 2017-01-21T00:00:46Z

-def log_timestamp(start_time, end_time, filename):
+def log_timestamp(start_time, end_time, filename = 'sconstruct.log'):
    '''Adds beginning and ending times to a log file.'''
    with open(filename, mode = 'r+U') as f:


Why do we use mode 'r+U' instead of our standard 'rU'?

Check out this wonderful chart!

arosenbe · 2017-01-21T22:44:34Z

See this comment on the R builder in the sister issue in gslab-econ/gslab_template#27.

qlquanle · 2017-02-09T23:00:13Z

Back on this after short vacation in TRANSville.

Conflicts: gslab_scons/builders/build_python.py gslab_scons/builders/build_r.py gslab_scons/builders/build_stata.py gslab_scons/misc.py gslab_scons/tests/log/make.log gslab_scons/tests/test_misc.py

qlquanle · 2017-02-10T02:53:49Z

Remaining action items

README update and additional documentation
Race conditions and log refactoring
Think about make_list_if_string()

qlquanle · 2017-02-11T00:25:10Z

@arosenbe what I understand from our discussion is that regarding to logging, we will:

keep outputting build logs with timestamp
reconstruct Sconstruct.log, adding to it builder logs. This is to avoid race conditions.
do NOT append SConstruct logs for each run, instead rewriting. Same for builder logs. This is to avoid merge headaches.

Let me know if you agree.

different appending mechanism from builder logs to sconstruct.log

qlquanle · 2017-02-17T22:25:43Z

@arosenbe

Regarding the concerns that were raised, here is how I chose to tackle them:

Tutorial on builder use: I think this is unnecessary given the current template for calling a builder:

source = '#source/analysis/analysis.do'
assets = '#output/data/data.txt'
env.Depends(target, assets)
env.Stata(target, source)

which is in response to the MG's desire to separate source code and assets in template.
In addition, please note that very little code in gslab_python was changed to accommodate this style (we still accept the "source code first" style). This is, IMO, so that we don't introduce unnecessary backward-incompatible changes.

Builder logs: The current construction is that the Sconstruct is rewritten with each run. For builders that are called during a Scons run, their Sconscript.log is appended to the Sconstruct.log using an atexit call, starting with most recent complete time. Since this is all atexit in the Sconstruct, there should be no concern regarding race conditions.
Make_list_if_string(): I enforce that input to Scons builder arguments are either str or list.

arosenbe

Nice work @stanfordquan. To your three points

Cool
I like this implementation a lot.
Great

I think there are some stray files lying around. Make sure to remove them before merging.

arosenbe · 2017-02-17T22:49:32Z

+    misc.check_code_extension(source_file, '.lyx')
+
+    # Set up target file and log file
+    newpdf      = source_file.replace('.lyx','.pdf')


Doesn't this handle edge.lyx.case.lyx poorly?

arosenbe · 2017-02-17T23:11:53Z

+                                shell  = True)
+    except subprocess.CalledProcessError:
+        message = misc.command_error_msg("Python", command)
+        raise BadExecutableError(message)


It doesn't look like BadExecutableError is loaded.

arosenbe · 2017-02-17T23:12:07Z

    log_timestamp(start_time, end_time, log_file)
+
    return None



Carriage Return.

arosenbe · 2017-02-17T23:30:18Z

+    builder_log_collect = {}
+
+    for root, dirs, files in os.walk(parent_dir): # take a walk in parent and child dirs
+        for file in files:


Let's not overwrite base python objects.

for f in files:

Good catch!

arosenbe · 2017-02-17T23:30:51Z

+        for file in files:
+            if file.endswith("sconscript.log"):
+                full_file = os.path.join(root, file)
+                with open(full_file, 'rU') as f:


We can make this f f_full.

arosenbe · 2017-02-17T23:31:17Z

+            if file.endswith("sconscript.log"):
+                full_file = os.path.join(root, file)
+                with open(full_file, 'rU') as f:
+                    s = f.readlines()[1] # skip a line


Why do we use the variable s?

resolved verbally

s short for string

arosenbe · 2017-02-17T23:32:24Z

+
+    # scan sconstruct.log for start time (if looks unpythonic, see xkcd 1171)
+    with open('sconstruct.log', "r") as f:
+        s = f.readline()


Why do we use the variable s here?

arosenbe · 2017-02-17T23:32:59Z


-def log_timestamp(start_time, end_time, filename):
+    with open('sconstruct.log', "a") as sconstruct:
+        for file in this_run_list:


Don't overwrite base python.

arosenbe · 2017-02-17T23:34:40Z

               'linux2': '-b'}
    option  = options[platform]
-    command = flavor + ' ' + option + ' %s ' + cl_arg # %s will take filename later
+    command = flavor + ' ' + option + ' %s %s'  # %s will take filename later


sprintf, please.

Resolved, @arosenbe happy to leave it the way it is.

arosenbe · 2017-02-17T23:34:59Z

    Windows platform.
    '''
-    command  = flavor + ' /e do' + ' %s ' + cl_arg # %s will take filename later
+    command  = flavor + ' /e do' + ' %s %s'  # %s will take filename later


sprintf, please

Resolved, @arosenbe happy to leave it the way it is.

arosenbe · 2017-02-18T00:06:36Z

@stanfordquan,

I'm getting the following error when trying to run the template. I think you're brining too much information into s in the logging functions.

(gslab-python/issue28_restructure) template$ scons 
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
scons: `.' is up to date.
scons: done building targets.
Error in atexit._run_exitfuncs:
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/atexit.py", line 24, in _run_exitfuncs
    func(*targs, **kargs)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/GSLab_Tools-3.0.4-py2.7.egg/gslab_scons/log.py", line 43, in end_log
    builder_logs = collect_builder_logs(parent_dir)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/GSLab_Tools-3.0.4-py2.7.egg/gslab_scons/log.py", line 79, in collect_builder_logs
    builder_log_end_time = datetime.strptime(s, "%Y-%m-%d %H:%M:%S")
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/_strptime.py", line 325, in _strptime
    (data_string, format))
ValueError: time data 'Log completed:  2017-02-06 14:31:02' does not match format '%Y-%m-%d %H:%M:%S'
Error in sys.exitfunc:
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/atexit.py", line 24, in _run_exitfuncs
    func(*targs, **kargs)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/GSLab_Tools-3.0.4-py2.7.egg/gslab_scons/log.py", line 43, in end_log
    builder_logs = collect_builder_logs(parent_dir)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/GSLab_Tools-3.0.4-py2.7.egg/gslab_scons/log.py", line 79, in collect_builder_logs
    builder_log_end_time = datetime.strptime(s, "%Y-%m-%d %H:%M:%S")
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/_strptime.py", line 325, in _strptime
    (data_string, format))
ValueError: time data 'Log completed:  2017-02-06 14:31:02' does not match format '%Y-%m-%d %H:%M:%S'

Figured out the issue. the new logger expects {} around the date, but scons saw that my build was up-to-date at the older gslab_python package. It didn't force a rebuild, so my log didn't have the {}

qlquanle · 2017-02-18T00:15:04Z

@arosenbe , your log files do not have the timestamp as specified, probably because you had a build folder.

This release is backward incompatible due to the fact that these logs must have timestamps.

arosenbe · 2017-02-18T00:23:58Z

Nice work @stanfordquan; good to merge.

qlquanle · 2017-02-18T00:24:01Z

@jmshapir @gentzkow

For gslab_python, some changes are backward incompatible in the sense that it requires an updated template to run.

We decided that it's good practice to direct these changes into a "dev_*" branch, where * is a new "major release" number in the semantic versioning. This seems to be what big projects do (e.g. here and here): directing issues into big milestones that are backward incompatible. This way we don't have to make backward incompatible changes in this repo and template more than, say, twice a year.

jmshapir · 2017-02-18T02:00:11Z

@stanfordquan I'm happy to defer to @gentzkow on that but it sounds reasonable to me.

* 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

Quan Le added 17 commits November 22, 2016 09:23

Merged master into origin/issue28-restructure

7b1b1a2

41fd030

#28 modify check_code_extension()

e34df8d

#28 make distinction between `target_dir` and `log_dir` for all but `build_stata()`, add code flow comments in builder

10ab85f

#28 minor change, `return None` switched to `return False` for logical check

Merge branch 'master'

982caa2

Conflicts: gslab_scons/tests/log/make.log

Merge branch 'master'

a298817

Conflicts: gslab_scons/tests/log/make.log

Merged branch master into issue28-restructure

a38d8ae

#28 add informative BadExecutableError messages

db61dfa

#28 merging remote to local

4796c91

Merged branch issue28-restructure into issue28-restructure

6e05c4d

Merged branch master into issue28-restructure

0018f25

#28 add single log file + appending mechanism

f54f8b2

#28 refactor build_stata()

99b4fb8

#28 add formatting for logs

5fe012a

#28 fix typo in docstring

5ab1c83

#28 update error messages and tests

b617bc1

#28 add mode check in start_log()

0d7fc57

qlquanle mentioned this pull request Jan 19, 2017

Pull request for gslab_python/#28: gslab_scons restructure gslab-econ/template#27

Merged

qlquanle requested a review from arosenbe January 19, 2017 19:59

arosenbe self-assigned this Jan 19, 2017

arosenbe suggested changes Jan 21, 2017

View reviewed changes

Quan Le added 2 commits February 9, 2017 15:10

Merge remote-tracking branch 'refs/remotes/origin/master'

ca966e7

Conflicts: gslab_scons/builders/build_python.py gslab_scons/builders/build_r.py gslab_scons/builders/build_stata.py gslab_scons/misc.py gslab_scons/tests/log/make.log gslab_scons/tests/test_misc.py

#28 change formatting in master tests to match work in this task

5997c3c

Quan Le added 2 commits February 10, 2017 16:32

#47 dev commit to work from home

a72e45b

#47 rework logging

8a831dd

different appending mechanism from builder logs to sconstruct.log

Quan Le added 2 commits February 17, 2017 14:11

#28 respond to PR

eb3e5db

#28 make pretty

5b93e40

#28 fix lyx pdf issue

fae0c79

arosenbe reviewed Feb 17, 2017

View reviewed changes

#28 respond to more comments

73360db

#28 fix typo

6fc12c3

qlquanle changed the base branch from master to dev_four February 18, 2017 00:14

qlquanle merged commit 796a8bd into dev_four Feb 18, 2017

qlquanle mentioned this pull request Feb 18, 2017

gslab_scons restructure #28

Closed

qlquanle deleted the issue28-restructure branch April 21, 2017 19:46

lboxell mentioned this pull request Jul 25, 2017

4.0.0 Release #81

Merged

qlquanle mentioned this pull request Jul 26, 2017

Find way to speed up log.py #85

Closed

Conversation

qlquanle commented Jan 19, 2017 • edited by arosenbe Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arosenbe commented Jan 19, 2017

Uh oh!

arosenbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arosenbe Jan 20, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arosenbe Jan 20, 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

qlquanle commented Feb 9, 2017

Uh oh!

qlquanle commented Feb 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qlquanle commented Feb 11, 2017

Uh oh!

qlquanle commented Feb 17, 2017

Uh oh!

arosenbe 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qlquanle commented Jan 19, 2017 •

edited by arosenbe

Loading

arosenbe Jan 20, 2017 •

edited

Loading

arosenbe Jan 20, 2017 •

edited

Loading

arosenbe commented Jan 21, 2017 •

edited

Loading

qlquanle commented Feb 10, 2017 •

edited

Loading

arosenbe commented Feb 18, 2017 •

edited

Loading