Fix #8155 - Improve API type checking and error handling, and produce Python traceback for Plugin #8181

jmarrec · 2020-07-31T14:10:19Z

Pull request overview

Fixes Enhanced Python API get_variable_value Error Handling #8155
Improve API type checking and error handling, and produce Python traceback for Plugin

I added a pytest file (and running that pytest as a cmake add_test) to show the error handling API side.

And here is a sample eplusout.err output with traceback:

Program Version,EnergyPlus, Version 9.4.0-d781d2b67d, YMD=2020.07.31 15:45,
   ************* PythonPlugin: Class AverageZoneTemps imported from: ../EnergyPlus/testfiles/PythonPluginCustomOutputVariable.py
   ************* Beginning Zone Sizing Calculations
   ** Severe  ** Call to on_end_of_zone_timestep_before_zone_reporting() on PythonPluginCustomOutputVariable.AverageZoneTemps failed!
   **   ~~~   ** Python error description follows: 
   **   ~~~   ** EnergyPlusException("`get_variable_handle` expects `component_type` as a `str` or UTF-8 encoded `bytes`, not '1'")
   **   ~~~   ** Python traceback follows: 
   **   ~~~   ** ```
   **   ~~~   **  >>> Traceback (most recent call last):
   **   ~~~   **  >>>   File "../EnergyPlus/testfiles/PythonPluginCustomOutputVariable.py", line 13, in on_end_of_zone_timestep_before_zone_reporting
    self.data['dummy_int_var_handle'] = self.api.exchange.get_variable_handle(1, "Environment")
   **   ~~~   **  >>>   File "/home/julien/Software/Others/EnergyPlus-build/Products/pyenergyplus/datatransfer.py", line 216, in get_variable_handle
    raise EnergyPlusException(
   **   ~~~   **  >>> pyenergyplus.common.EnergyPlusException: `get_variable_handle` expects `component_type` as an `str` or `bytes`, not '1'
   **   ~~~   ** ```
   **  Fatal  ** Program terminates after call to on_end_of_zone_timestep_before_zone_reporting() on PythonPluginCustomOutputVariable.AverageZoneTemps failed!
   ...Summary of Errors that led to program termination:
   ..... Reference severe error count=1
   ..... Last severe error=Call to on_end_of_zone_timestep_before_zone_reporting() on PythonPluginCustomOutputVariable.AverageZoneTemps failed!
   ************* Warning:  Node connection errors not checked - most system input has not been read (see previous warning).
   ************* Fatal error -- final processing.  Program exited before simulations began.  See previous error messages.
   ************* EnergyPlus Warmup Error Summary. During Warmup: 0 Warning; 0 Severe Errors.
   ************* EnergyPlus Sizing Error Summary. During Sizing: 0 Warning; 1 Severe Errors.
   ************* EnergyPlus Terminated--Fatal Error Detected. 0 Warning; 1 Severe Errors; Elapsed Time=00hr 00min  3.97sec

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

Reviewer

This will not be exhaustively relevant to every PR.

… a RealEP

…lusout.err.

…ly copied to the Products/pyenergyplus dir on rebuild temporary*: this will ALWAYS run, even if no changes were done. But these are small .py files and no targets will be recompiled because of it, so perhaps it's acceptable as is

jmarrec · 2020-07-31T14:11:03Z

src/EnergyPlus/CMakeLists.txt

+        # TODO: temporary "hack" to depend on a specific file that doesn't exist to force this to run
+        # ideally the whole process would be revamped
+        DEPENDS __ALWAYSRUNME


@Myoldmopar FYI. This is unrelated to #8155 but we've talked about it.

A nice workaround for right now!

src/EnergyPlus/CMakeLists.txt

jmarrec · 2020-07-31T14:15:00Z

src/EnergyPlus/PythonLibWrapper.cc

+    PYTHONWRAP_API PyObject * EP_Wrap_PyObject_CallFunction3Args(PyObject *callable, const char *format, PyObject * arg1, PyObject * arg2, PyObject * arg3) {return PyObject_CallFunction(callable, format, arg1, arg2, arg3);}
+


I add to register this one so I could call a function with 3 args and a format string.

PyObject_CallFunction is actually variadic, something the wrapping is losing. I couldn't really figure out how to forward these variable number of args, so I just added a wrap for my specific number of args that I needed.

Note the ... in the function signature:

PyObject* PyObject_CallFunction(PyObject *callable, const char *format, ...)

https://docs.python.org/3/c-api/object.html#c.PyObject_CallFunction

Understood, I don't imagine we'll need any additional instances of CallFunction, so this is a suitable addition.

jmarrec · 2020-07-31T14:15:32Z

src/EnergyPlus/PythonLibWrapper.cc

@@ -70,6 +70,7 @@ extern "C" {
    PYTHONWRAP_API int EP_Wrap_PyRun_SimpleString(const char * c) {return PyRun_SimpleString(c);}
    PYTHONWRAP_API int EP_Wrap_Py_FinalizeEx() {return Py_FinalizeEx();}
    PYTHONWRAP_API void EP_Wrap_PyErr_Fetch(PyObject **a, PyObject **b, PyObject **c) {PyErr_Fetch(a, b, c);}
+    PYTHONWRAP_API void EP_Wrap_PyErr_NormalizeException(PyObject **a, PyObject **b, PyObject **c) {PyErr_NormalizeException(a, b, c);}


This is needed as I got a few cases where without it I couldn't produce a traceback.

jmarrec · 2020-07-31T14:16:31Z

src/EnergyPlus/api/datatransfer.py

        if isinstance(variable_name, str):
            variable_name = variable_name.encode('utf-8')
+        elif not isinstance(variable_name, bytes):
+            raise EnergyPlusException(
+                "`request_variable` expects `component_type` as a `str` or UTF-8 encoded `bytes`, not "
+                "'{}'".format(variable_name))


Typical Type checking for a Union[str, bytes] argument

jmarrec · 2020-07-31T14:17:12Z

src/EnergyPlus/api/datatransfer.py

        return self.api.getActuatorHandle(component_type, control_type, actuator_key)

-    def get_variable_value(self, variable_handle: int) -> RealEP:
+    def get_variable_value(self, variable_handle: int) -> float:


I changed the type hints for the entire file to use float and not RealEP as it should be.

jmarrec · 2020-07-31T14:17:28Z

src/EnergyPlus/api/datatransfer.py

+        if not is_number(variable_handle):
+            raise EnergyPlusException(
+                "`get_variable_value` expects `variable_handle` as an `int`, not "
+                "'{}'".format(variable_handle))


Typical type checking for an int argument

jmarrec · 2020-07-31T14:17:41Z

src/EnergyPlus/api/datatransfer.py

+        if not is_number(actuator_value):
+            raise EnergyPlusException(
+                "`set_actuator_value` expects `actuator_value` as a `float`, not "
+                "'{}'".format(actuator_value))


Typical type checking for a float argument

jmarrec · 2020-07-31T14:18:20Z

tst/EnergyPlus/api/test_DataTransferTypes.py

+import pytest
+
+from pyenergyplus.api import EnergyPlusAPI
+from pyenergyplus.common import EnergyPlusException
+
+api = EnergyPlusAPI()
+
+class TestPythonAPITypes():
+    """
+    py.test on EnergyPlusAPI.exchange functions to see type checking in action
+    """
+
+    def test_get_actuator_handle(self):
+
+        assert (-1 == api.exchange.get_actuator_handle(
+            u"Weather Data", u"Outdoor Dew Point", u"Environment"))
+
+        # Wrong number of arguments
+        with pytest.raises(TypeError) as e:
+            api.exchange.get_actuator_handle(
+                u"Weather Data", u"Outdoor Dew Point")
+            assert ("get_actuator_handle() missing 1 required positional argument: 'actuator_key'" in str(e.value))
+
+        with pytest.raises(EnergyPlusException) as e:
+            api.exchange.get_actuator_handle(
+                1, u"Outdoor Dew Point", u"Environment")
+            assert ("`get_actuator_handle` expects `component_type` as a `str` or UTF-8 encoded `bytes`, not '1'" in str(e.value))
+
+        with pytest.raises(EnergyPlusException) as e:
+            api.exchange.get_actuator_handle(
+                u"Weather Data", 2, u"Environment")
+            assert ("`get_actuator_handle` expects `control_type` as a `str` or UTF-8 encoded `bytes`, not '2'" in str(e.value))
+
+        with pytest.raises(EnergyPlusException) as e:
+            api.exchange.get_actuator_handle(
+                u"Weather Data", u"Outdoor Dew Point", 3)
+            assert ("`get_actuator_handle` expects `actuator_key` as a `str` or UTF-8 encoded `bytes`, not '3'" in str(e.value))
+
+
+    def test_get_variable_value(self):
+        assert(0.0 == api.exchange.get_variable_value(-1))
+
+        with pytest.raises(TypeError) as e:
+            api.exchange.get_variable_value()
+            assert ("get_variable_value() missing 1 required positional argument: 'variable_handle'" in str(e.value))
+
+        with pytest.raises(EnergyPlusException) as e:
+            api.exchange.get_variable_value(u"foo")
+            assert ("`get_variable_value` expects `variable_handle` as a `str` or UTF-8 encoded `bytes`, not 'foo'" in str(e.value))


New pytest file to show behavior for a couple of functions each.

jmarrec · 2020-07-31T14:19:24Z

src/EnergyPlus/PluginManager.cc

+
+        // See if we can get a full traceback.
+        // Calls into python, and does the same as capturing the exception in `e`
+        // then `print(traceback.format_exception(e.type, e.value, e.tb))`
+        PyObjectWrap pModuleName = (*EP_PyUnicode_DecodeFSDefault)("traceback");
+        PyObjectWrap pyth_module = (*EP_PyImport_Import)(pModuleName);
+        (*EP_Py_DECREF)(pModuleName);
+
+        if (pyth_module == NULL) {
+            EnergyPlus::ShowFatalError("Cannot find 'traceback' module in reportPythonError(), this is weird");
+            return;
+        }
+
+        PyObjectWrap pyth_func = (*EP_PyObject_GetAttrString)(pyth_module, "format_exception");
+        (*EP_Py_DECREF)(pyth_module); // PyImport_Import returns a new reference, decrement it
+
+        if (pyth_func || (*EP_PyCallable_Check)(pyth_func)) {
+
+            PyObjectWrap pyth_val = (*EP_PyObject_CallFunction3Args)(pyth_func, "OOO", exc_type, exc_value, exc_tb);
+
+            // traceback.format_exception returns a list, so iterate on that
+            if (!pyth_val || !(*EP_PyList_Check)(pyth_val)) { // NOLINT(hicpp-signed-bitwise)
+                EnergyPlus::ShowFatalError("In reportPythonError(), traceback.format_exception did not return a list.");
+            }
+
+            unsigned long numVals = (*EP_PyList_Size)(pyth_val);
+            if (numVals == 0) {
+                EnergyPlus::ShowFatalError("No traceback available");
+                return;
+            }
+
+            EnergyPlus::ShowContinueError("Python traceback follows: ");
+
+            EnergyPlus::ShowContinueError("```");
+
+            for (unsigned long itemNum = 0; itemNum < numVals; itemNum++) {
+                PyObjectWrap item = (*EP_PyList_GetItem)(pyth_val, itemNum);
+                if ((*EP_PyUnicode_Check)(item)) { // NOLINT(hicpp-signed-bitwise) -- something inside Python code causes warning
+                    std::string traceback_line = (*EP_PyUnicode_AsUTF8)(item);
+                    if (!traceback_line.empty() && traceback_line[traceback_line.length()-1] == '\n') {
+                        traceback_line.erase(traceback_line.length()-1);
+                    }
+                    EnergyPlus::ShowContinueError(" >>> " + traceback_line);
+                }
+                // PyList_GetItem returns a borrowed reference, do not decrement
+            }
+
+            EnergyPlus::ShowContinueError("```");
+
+            // PyList_Size returns a borrowed reference, do not decrement
+            (*EP_Py_DECREF)(pyth_val); // PyObject_CallFunction returns new reference, decrement
+        }
+        (*EP_Py_DECREF)(pyth_func); // PyObject_GetAttrString returns a new reference, decrement it


New call from C to Python to get an actual traceback, then decode every line in said traceback and output it to the eplusout.err

This is very cool.

jmarrec · 2020-08-03T04:14:53Z

Mac bot is reporting that it doesn't have pytest installed :/

/usr/local/opt/python@3.8/bin/python3.8: No module named pytest

Myoldmopar · 2020-08-06T14:15:24Z

Can you just use the raw unittest package in the standard library?

Myoldmopar

The only thing that I think really needs to change is pytest -> unittest. Otherwise this is great!

Myoldmopar · 2020-08-07T18:32:21Z

src/EnergyPlus/CMakeLists.txt

+        # TODO: temporary "hack" to depend on a specific file that doesn't exist to force this to run
+        # ideally the whole process would be revamped
+        DEPENDS __ALWAYSRUNME


A nice workaround for right now!

src/EnergyPlus/CMakeLists.txt

Myoldmopar · 2020-08-07T18:34:56Z

src/EnergyPlus/PluginManager.cc

+
+        // See if we can get a full traceback.
+        // Calls into python, and does the same as capturing the exception in `e`
+        // then `print(traceback.format_exception(e.type, e.value, e.tb))`
+        PyObjectWrap pModuleName = (*EP_PyUnicode_DecodeFSDefault)("traceback");
+        PyObjectWrap pyth_module = (*EP_PyImport_Import)(pModuleName);
+        (*EP_Py_DECREF)(pModuleName);
+
+        if (pyth_module == NULL) {
+            EnergyPlus::ShowFatalError("Cannot find 'traceback' module in reportPythonError(), this is weird");
+            return;
+        }
+
+        PyObjectWrap pyth_func = (*EP_PyObject_GetAttrString)(pyth_module, "format_exception");
+        (*EP_Py_DECREF)(pyth_module); // PyImport_Import returns a new reference, decrement it
+
+        if (pyth_func || (*EP_PyCallable_Check)(pyth_func)) {
+
+            PyObjectWrap pyth_val = (*EP_PyObject_CallFunction3Args)(pyth_func, "OOO", exc_type, exc_value, exc_tb);
+
+            // traceback.format_exception returns a list, so iterate on that
+            if (!pyth_val || !(*EP_PyList_Check)(pyth_val)) { // NOLINT(hicpp-signed-bitwise)
+                EnergyPlus::ShowFatalError("In reportPythonError(), traceback.format_exception did not return a list.");
+            }
+
+            unsigned long numVals = (*EP_PyList_Size)(pyth_val);
+            if (numVals == 0) {
+                EnergyPlus::ShowFatalError("No traceback available");
+                return;
+            }
+
+            EnergyPlus::ShowContinueError("Python traceback follows: ");
+
+            EnergyPlus::ShowContinueError("```");
+
+            for (unsigned long itemNum = 0; itemNum < numVals; itemNum++) {
+                PyObjectWrap item = (*EP_PyList_GetItem)(pyth_val, itemNum);
+                if ((*EP_PyUnicode_Check)(item)) { // NOLINT(hicpp-signed-bitwise) -- something inside Python code causes warning
+                    std::string traceback_line = (*EP_PyUnicode_AsUTF8)(item);
+                    if (!traceback_line.empty() && traceback_line[traceback_line.length()-1] == '\n') {
+                        traceback_line.erase(traceback_line.length()-1);
+                    }
+                    EnergyPlus::ShowContinueError(" >>> " + traceback_line);
+                }
+                // PyList_GetItem returns a borrowed reference, do not decrement
+            }
+
+            EnergyPlus::ShowContinueError("```");
+
+            // PyList_Size returns a borrowed reference, do not decrement
+            (*EP_Py_DECREF)(pyth_val); // PyObject_CallFunction returns new reference, decrement
+        }
+        (*EP_Py_DECREF)(pyth_func); // PyObject_GetAttrString returns a new reference, decrement it


This is very cool.

Myoldmopar · 2020-08-07T18:35:33Z

src/EnergyPlus/PythonLibWrapper.cc

+    PYTHONWRAP_API PyObject * EP_Wrap_PyObject_CallFunction3Args(PyObject *callable, const char *format, PyObject * arg1, PyObject * arg2, PyObject * arg3) {return PyObject_CallFunction(callable, format, arg1, arg2, arg3);}
+


Understood, I don't imagine we'll need any additional instances of CallFunction, so this is a suitable addition.

Myoldmopar · 2020-08-07T18:36:06Z

tst/EnergyPlus/api/test_DataTransferTypes.py

+import pytest
+
+from pyenergyplus.api import EnergyPlusAPI
+from pyenergyplus.common import EnergyPlusException
+
+api = EnergyPlusAPI()
+
+class TestPythonAPITypes():
+    """
+    py.test on EnergyPlusAPI.exchange functions to see type checking in action
+    """
+
+    def test_get_actuator_handle(self):
+
+        assert (-1 == api.exchange.get_actuator_handle(
+            u"Weather Data", u"Outdoor Dew Point", u"Environment"))
+
+        # Wrong number of arguments
+        with pytest.raises(TypeError) as e:
+            api.exchange.get_actuator_handle(
+                u"Weather Data", u"Outdoor Dew Point")
+            assert ("get_actuator_handle() missing 1 required positional argument: 'actuator_key'" in str(e.value))
+
+        with pytest.raises(EnergyPlusException) as e:
+            api.exchange.get_actuator_handle(
+                1, u"Outdoor Dew Point", u"Environment")
+            assert ("`get_actuator_handle` expects `component_type` as a `str` or UTF-8 encoded `bytes`, not '1'" in str(e.value))
+
+        with pytest.raises(EnergyPlusException) as e:
+            api.exchange.get_actuator_handle(
+                u"Weather Data", 2, u"Environment")
+            assert ("`get_actuator_handle` expects `control_type` as a `str` or UTF-8 encoded `bytes`, not '2'" in str(e.value))
+
+        with pytest.raises(EnergyPlusException) as e:
+            api.exchange.get_actuator_handle(
+                u"Weather Data", u"Outdoor Dew Point", 3)
+            assert ("`get_actuator_handle` expects `actuator_key` as a `str` or UTF-8 encoded `bytes`, not '3'" in str(e.value))
+
+
+    def test_get_variable_value(self):
+        assert(0.0 == api.exchange.get_variable_value(-1))
+
+        with pytest.raises(TypeError) as e:
+            api.exchange.get_variable_value()
+            assert ("get_variable_value() missing 1 required positional argument: 'variable_handle'" in str(e.value))
+
+        with pytest.raises(EnergyPlusException) as e:
+            api.exchange.get_variable_value(u"foo")
+            assert ("`get_variable_value` expects `variable_handle` as a `str` or UTF-8 encoded `bytes`, not 'foo'" in str(e.value))


Myoldmopar · 2020-08-07T18:36:54Z

This is a really great cleanup on the Python type handling, and the traceback is super nice. I understand the issue with the variadics, and while I don't like the intermediate Python "wrapper", it was very important to create a lazy link to the Python DLL. I can go into more detail about that whole process some other time. In any case, this looks good outside of the pytest issue. How do you feel about making the change to just raw unittest?

jmarrec · 2020-08-07T20:45:48Z

I'm nor too familiar with unittest, but it shoud be straigjtforward to migrate since it's a small test file. I'll do that then, I know how much you don't want to add python module dependencies :)
I thought pytest was part of the standard python library in 3.x honestly

…rorHandling

Myoldmopar

Everything looks great now! This is a big set of fixes, thanks!

Myoldmopar · 2020-08-12T20:07:29Z

tst/EnergyPlus/api/test_DataTransferTypes.py

@@ -0,0 +1,53 @@
+import unittest


Myoldmopar · 2020-08-12T20:09:03Z

CI is very happy now with the latest test. I'm going to pull in develop locally and make sure it's all good since develop has moved a lot today, but I anticipate it is ready to go. Thanks!

Myoldmopar · 2020-08-12T21:38:10Z

Everything passes locally, this is great. Thanks @jmarrec !

jmarrec · 2020-08-13T06:35:16Z

@Myoldmopar thanks for the review. Should we do something about the duplicate actuators in some the EMS & PythonPlugin EMS clones that are now issuing a warning in the eplusout.err?

jmarrec added 7 commits July 31, 2020 14:45

Update type hints: everything in python (user facing) is a float, not…

d781d2b

… a RealEP

Add actual type checking for numeric types

57c0bcd

Add actual type checking for [str, bytes] arguments

b479219

Clean up docstring of a function that is probably a copy paste

46e7321

Decode and format the actual Python traceback to print that to the ep…

4e0af58

…lusout.err.

Add a pytest file to showcase the new error checking in datatransfer.py

74b2f13

jmarrec added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Jul 31, 2020

jmarrec requested a review from Myoldmopar July 31, 2020 14:10

jmarrec self-assigned this Jul 31, 2020

jmarrec commented Jul 31, 2020

View reviewed changes

Myoldmopar requested changes Aug 7, 2020

View reviewed changes

jmarrec added 2 commits August 11, 2020 14:39

Merge remote-tracking branch 'origin/develop' into 8155_Python_TypeEr…

9a90197

…rorHandling

Port test file from pytest to unittest

f6829d8

jmarrec requested a review from Myoldmopar August 11, 2020 13:06

Myoldmopar approved these changes Aug 12, 2020

View reviewed changes

tst/EnergyPlus/api/test_DataTransferTypes.py

@@ -0,0 +1,53 @@

import unittest

Copy link

Member

Myoldmopar Aug 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Myoldmopar merged commit ea99180 into develop Aug 12, 2020

Myoldmopar deleted the 8155_Python_TypeErrorHandling branch August 12, 2020 21:38

mitchute mentioned this pull request Sep 10, 2020

Continued API/Plugin Issues #7877

Closed

11 tasks

jmarrec mentioned this pull request Nov 10, 2023

Fix #5018 - C++ CLI: Calling a non existing method in a measure ends up with a crash and stack trace NREL/OpenStudio#5023

Merged

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #8155 - Improve API type checking and error handling, and produce Python traceback for Plugin #8181

Fix #8155 - Improve API type checking and error handling, and produce Python traceback for Plugin #8181

jmarrec commented Jul 31, 2020

jmarrec Jul 31, 2020

Myoldmopar Aug 7, 2020

jmarrec Jul 31, 2020

Myoldmopar Aug 7, 2020

jmarrec Jul 31, 2020

jmarrec Jul 31, 2020

jmarrec Jul 31, 2020

jmarrec Jul 31, 2020

jmarrec Jul 31, 2020

jmarrec Jul 31, 2020

Myoldmopar Aug 7, 2020

jmarrec Jul 31, 2020

Myoldmopar Aug 7, 2020

jmarrec commented Aug 3, 2020

Myoldmopar commented Aug 6, 2020 •

edited

Myoldmopar left a comment

Myoldmopar Aug 7, 2020

Myoldmopar Aug 7, 2020

Myoldmopar Aug 7, 2020

Myoldmopar Aug 7, 2020

Myoldmopar commented Aug 7, 2020

jmarrec commented Aug 7, 2020

Myoldmopar left a comment

Myoldmopar Aug 12, 2020

Myoldmopar commented Aug 12, 2020

Myoldmopar commented Aug 12, 2020

jmarrec commented Aug 13, 2020

		PYTHONWRAP_API PyObject * EP_Wrap_PyObject_CallFunction3Args(PyObject callable, const char format, PyObject * arg1, PyObject * arg2, PyObject * arg3) {return PyObject_CallFunction(callable, format, arg1, arg2, arg3);}

Fix #8155 - Improve API type checking and error handling, and produce Python traceback for Plugin #8181

Fix #8155 - Improve API type checking and error handling, and produce Python traceback for Plugin #8181

Conversation

jmarrec commented Jul 31, 2020

Pull request overview

Pull Request Author

Reviewer

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

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

jmarrec commented Aug 3, 2020

Myoldmopar commented Aug 6, 2020 • edited

Myoldmopar 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

Myoldmopar commented Aug 7, 2020

jmarrec commented Aug 7, 2020

Myoldmopar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar commented Aug 12, 2020

Myoldmopar commented Aug 12, 2020

jmarrec commented Aug 13, 2020

Myoldmopar commented Aug 6, 2020 •

edited