Per-node web temp directory #902

2020-11-18T23:17:45Z

exarkun commented

2020-11-18 23:17:45 +00:00

(Migrated from github.com)

https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1549

codecov[bot] commented

2020-11-18 23:44:25 +00:00

(Migrated from github.com)

Codecov Report

Merging #902 (520f4d1) into master (dc611bf) will increase coverage by 0%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master    #902     +/-   ##
========================================
  Coverage      92%     93%             
========================================
  Files         158     158             
  Lines       27581   29102   +1521     
  Branches     4131    4546    +415     
========================================
+ Hits        25421   26920   +1499     
- Misses       1487    1504     +17     
- Partials      673     678      +5

Impacted Files	Coverage Δ
src/allmydata/node.py	`98% <ø> (+2%)`	⬆️
src/allmydata/client.py	`95% <100%> (+1%)`	⬆️
src/allmydata/webish.py	`88% <100%> (+1%)`	⬆️
src/allmydata/version_checks.py	`83% <0%> (-1%)`	⬇️
src/allmydata/util/log.py	`100% <0%> (ø)`
src/allmydata/util/_python3.py	`100% <0%> (ø)`
src/allmydata/mutable/retrieve.py	`97% <0%> (+1%)`	⬆️
src/allmydata/mutable/servermap.py	`95% <0%> (+1%)`	⬆️
... and 6 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 dc611bf...520f4d1. Read the comment docs.

# [Codecov](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902?src=pr&el=h1) Report > Merging [#902](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902?src=pr&el=desc) (520f4d1) into [master](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/commit/dc611bf9b27ce81b719231ccffa61dbb699012d1?el=desc) (dc611bf) will **increase** coverage by `0%`. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902/graphs/tree.svg?width=650&height=150&src=pr&token=Ztmu9sr4vi)](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #902 +/- ## ======================================== Coverage 92% 93% ======================================== Files 158 158 Lines 27581 29102 +1521 Branches 4131 4546 +415 ======================================== + Hits 25421 26920 +1499 - Misses 1487 1504 +17 - Partials 673 678 +5 ``` | [Impacted Files](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [src/allmydata/node.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9ub2RlLnB5) | `98% <ø> (+2%)` | :arrow_up: | | [src/allmydata/client.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9jbGllbnQucHk=) | `95% <100%> (+1%)` | :arrow_up: | | [src/allmydata/webish.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS93ZWJpc2gucHk=) | `88% <100%> (+1%)` | :arrow_up: | | [src/allmydata/version\_checks.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS92ZXJzaW9uX2NoZWNrcy5weQ==) | `83% <0%> (-1%)` | :arrow_down: | | [src/allmydata/util/log.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS91dGlsL2xvZy5weQ==) | `100% <0%> (ø)` | | | [src/allmydata/util/\_python3.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS91dGlsL19weXRob24zLnB5) | `100% <0%> (ø)` | | | [src/allmydata/mutable/retrieve.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9tdXRhYmxlL3JldHJpZXZlLnB5) | `97% <0%> (+1%)` | :arrow_up: | | [src/allmydata/mutable/servermap.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9tdXRhYmxlL3NlcnZlcm1hcC5weQ==) | `95% <0%> (+1%)` | :arrow_up: | | ... and [6 more](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902/diff?src=pr&el=tree-more) | | ------ [Continue to review full report at Codecov](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902?src=pr&el=footer). Last update [dc611bf...520f4d1](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/902?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

meejah (Migrated from github.com) approved these changes 2020-11-25 18:55:51 +00:00

meejah (Migrated from github.com) left a comment

👍 looks good, some inline comments/questions

:+1: looks good, some inline comments/questions

src/allmydata/test/web/test_webish.py

meejah (Migrated from github.com) commented

2020-11-25 18:45:24 +00:00

Aaaaaaaaa, this was a class name??

src/allmydata/test/web/test_webish.py

meejah (Migrated from github.com) commented

2020-11-25 18:46:33 +00:00

possibly "_create_request" since it makes a new one every time?

src/allmydata/test/web/test_webish.py

meejah (Migrated from github.com) commented

2020-11-25 18:50:19 +00:00

Not necessarily asking for changes, but why not just 1024*1024+1? What do we gain by trying a bunch of random sizes (instead of just one that we know is big enough)?

Not necessarily asking for changes, but why not just `1024*1024+1`? What do we gain by trying a bunch of random sizes (instead of just one that we know is big enough)?

exarkun (Migrated from github.com) reviewed 2020-11-25 21:09:25 +00:00

src/allmydata/test/web/test_webish.py

exarkun (Migrated from github.com) commented

2020-11-25 21:09:25 +00:00

Fortunately just a factory function not a class (or ... I dunno ... a partial application of a class?)

exarkun (Migrated from github.com) reviewed 2020-11-25 21:11:41 +00:00

src/allmydata/test/web/test_webish.py

exarkun (Migrated from github.com) commented

2020-11-25 21:11:41 +00:00

It's a more complete representation of the specification. We want anything this size or larger to behave this way, not just a single value. We do know now (at least we think we do) that all values at least this large behave the same ... but we might be wrong or the implementation might change in the future to invalidate this knowledge.

It's a more complete representation of the specification. We want anything this size or larger to behave this way, not just a single value. We do know *now* (at least we think we do) that all values at least this large behave the same ... but we might be wrong or the implementation might change in the future to invalidate this knowledge.