tahoe-lafs/tahoe-lafs
tahoe-lafs
/
tahoe-lafs
2
0
Fork 0
Code Issues Pull Requests 23 Packages Projects Releases 1 Wiki Activity

Port the rest of allmydata.immutable to Python 3 #922

Merged
itamarst merged 5 commits from 3551.more-immutable-python-3 into master 2020-12-10 15:46:43 +00:00
Conversation 1 Commits 5 Files Changed 6 +49 -10
itamarst commented 2020-12-09 18:33:43 +00:00 (Migrated from github.com)
Copy Link

Fixes https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3551

Fixes https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3551
codecov[bot] commented 2020-12-09 19:37:51 +00:00 (Migrated from github.com)
Copy Link

Codecov Report

Merging #922 (e9b0a52) into master (c1edd7a) will increase coverage by 0%.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #922   +/-   ##
======================================
  Coverage      92%     92%           
======================================
  Files         157     157           
  Lines       27508   27522   +14     
  Branches     4115    4117    +2     
======================================
+ Hits        25295   25318   +23     
+ Misses       1545    1538    -7     
+ Partials      668     666    -2
Impacted Files Coverage Δ
src/allmydata/util/_python3.py 100% <ø> (ø)
src/allmydata/immutable/checker.py 89% <100%> (+<1%) ⬆️
src/allmydata/immutable/repairer.py 100% <100%> (ø)
src/allmydata/web/status.py 95% <0%> (-<1%) ⬇️
src/allmydata/immutable/downloader/share.py 95% <0%> (+2%) ⬆️

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 c1edd7a...e9b0a52. Read the comment docs.

# [Codecov](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922?src=pr&el=h1) Report > Merging [#922](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922?src=pr&el=desc) (e9b0a52) into [master](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/commit/c1edd7a56856be30b338e40b01318a00433dcddb?el=desc) (c1edd7a) will **increase** coverage by `0%`. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922/graphs/tree.svg?width=650&height=150&src=pr&token=Ztmu9sr4vi)](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #922 +/- ## ====================================== Coverage 92% 92% ====================================== Files 157 157 Lines 27508 27522 +14 Branches 4115 4117 +2 ====================================== + Hits 25295 25318 +23 + Misses 1545 1538 -7 + Partials 668 666 -2 ``` | [Impacted Files](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [src/allmydata/util/\_python3.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS91dGlsL19weXRob24zLnB5) | `100% <ø> (ø)` | | | [src/allmydata/immutable/checker.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9pbW11dGFibGUvY2hlY2tlci5weQ==) | `89% <100%> (+<1%)` | :arrow_up: | | [src/allmydata/immutable/repairer.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9pbW11dGFibGUvcmVwYWlyZXIucHk=) | `100% <100%> (ø)` | | | [src/allmydata/web/status.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS93ZWIvc3RhdHVzLnB5) | `95% <0%> (-<1%)` | :arrow_down: | | [src/allmydata/immutable/downloader/share.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9pbW11dGFibGUvZG93bmxvYWRlci9zaGFyZS5weQ==) | `95% <0%> (+2%)` | :arrow_up: | ------ [Continue to review full report at Codecov](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922?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/922?src=pr&el=footer). Last update [c1edd7a...e9b0a52](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/922?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
exarkun (Migrated from github.com) approved these changes 2020-12-09 20:19:12 +00:00
exarkun (Migrated from github.com) left a comment
Copy Link

Thanks. Looks good to me. One semi-inline comment to think about. Merge when you're satisfied.

Thanks. Looks good to me. One semi-inline comment to think about. Merge when you're satisfied.
src/allmydata/immutable/checker.py
@ -1,0 +9,4 @@
from future.utils import PY2
if PY2:
from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401
exarkun (Migrated from github.com) commented 2020-12-09 19:31:50 +00:00
Copy Link

There's a __str__ below that calls a to_string() method and interpolates the result. I think the to_string is from allmydata/uri.py and the module docstring there says it returns bytes.

Maybe that's all fine? It just means there'll be some extra b'...' stuff in the result. And I don't even know what these objects are or why you'd see them, so I guess that's fine.

There's a similar thing for a prefix computed for log.PrefixingLogMixin but I guess the same reasoning applies.

There's a `__str__` below that calls a `to_string()` method and interpolates the result. I think the `to_string` is from `allmydata/uri.py` and the module docstring there says it returns bytes. Maybe that's all fine? It just means there'll be some extra b'...' stuff in the result. And I don't even know what these objects are or why you'd see them, so I guess that's fine. There's a similar thing for a prefix computed for log.PrefixingLogMixin but I guess the same reasoning applies.
itamarst (Migrated from github.com) reviewed 2020-12-10 15:06:38 +00:00
src/allmydata/immutable/checker.py
@ -1,0 +9,4 @@
from future.utils import PY2
if PY2:
from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401
itamarst (Migrated from github.com) commented 2020-12-10 15:06:38 +00:00
Copy Link

I didn't notice this one, but made the explicit decision in a later PR that yeah, if it's just debug logs or whatever, switching to bytes or repr() instead of custom-massaged string formatting of byte sequences etc. is fine. A human can interpret what's going on reasonably.

I didn't notice this one, but made the explicit decision in a later PR that yeah, if it's just debug logs or whatever, switching to bytes or `repr()` instead of custom-massaged string formatting of byte sequences etc. is fine. A human can interpret what's going on reasonably.
Sign in to join this conversation.
Reviewers
No reviewers
exarkun
Labels
Clear labels   
Benchmarking and Performance

   
HTTP Storage Protocol

   
Nevow Removal

   
Python 3 Porting

   
not-for-merge
No Label
Benchmarking and Performance
HTTP Storage Protocol
Nevow Removal
Python 3 Porting
not-for-merge
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: tahoe-lafs/tahoe-lafs#922
No description provided.
Delete Branch "3551.more-immutable-python-3"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?