increase Request.prof_file max_length to 300 #203
Conversation
|
i guess that technically, this doesn't fix the underlying issue of #201, but rather kicks the can down the road. It's still possible that a DataError would be raised, and the profiler results not saved to the db, if the path is too long. And it's not immediately apparent from the traceback why that is. |
|
@smcoll Could you add a small test for this change when you have a moment? I saw the failure and it's fine. It's just a precision error. |
|
@avelis a test which asserts that the path can be up to a certain length? |
|
@smcoll A test can assert the length or the failure if going past 300. Nothing to fancy |
|
@avelis what is the ideal behavior if the path is too long, or if the file can't be saved? |
|
@smcoll That raises a good question.
However, I feel that introduces new/different scope into this PR. If you prefer, I can merge it in as is without issue. Thoughts? |
|
@avelis that seems reasonable to me. The underlying issue about how to handle a file storage failure is not addressed in this PR, so i changed the description to relate to but not fix the issue. A PR which actually changes the behavior of the app to handle storage failures seems like a better candidate for new tests. |
|
@smcoll Thanks for the contribution! I appreciate it. |
* move ProfilerResultStorage to own module to resolve import * use SilkyConfig for SILKY_STORAGE_CLASS * terse storage definition * didn't mean to reorder imports * increase Request.prof_file max_length to 300 (#203) * test default storage class
* move ProfilerResultStorage to own module to resolve import * use SilkyConfig for SILKY_STORAGE_CLASS * terse storage definition * didn't mean to reorder imports * increase Request.prof_file max_length to 300 (jazzband#203) * test default storage class
references #201