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

Add mypy support #915

Merged
jaraco merged 84 commits from 3399.mypy into master 2021-01-15 21:00:58 +00:00
Conversation 7 Commits 84 Files Changed 50 +276 -131
jaraco commented 2020-12-04 13:50:37 +00:00 (Migrated from github.com)
Copy Link

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

https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3399
jaraco commented 2020-12-05 01:00:19 +00:00 (Migrated from github.com)
Copy Link

This test run demonstrates the checks now passing in CI.

[This test run](https://app.circleci.com/pipelines/github/tahoe-lafs/tahoe-lafs/1496/workflows/77d849e7-e809-488b-bc12-718e99ee3ca2/jobs/38640) demonstrates the checks now passing in CI.
exarkun (Migrated from github.com) requested changes 2020-12-15 18:16:16 +00:00
exarkun (Migrated from github.com) left a comment
Copy Link

Thanks. Left a bunch of comments inline. Some additional thoughts:

  • There are some conflicts with master now, alas.
  • CI isn't completely happy.
  • Are we going to need a ratchet-style job for mypy checks at some point? It seems like at the end of this PR we have mypy running with no errors but, eg, are a ton of errors going to appear when we bring in the type stubs for our dependencies? Since that's later it doesn't need to be a blocker for this PR, I'm just wondering - and if there's anything we can do now to make our lives easier then it might be worth it.
Thanks. Left a bunch of comments inline. Some additional thoughts: * There are some conflicts with master now, alas. * CI isn't completely happy. * Are we going to need a ratchet-style job for mypy checks at some point? It seems like at the end of *this* PR we have mypy running with no errors but, eg, are a ton of errors going to appear when we bring in the type stubs for our dependencies? Since that's later it doesn't need to be a blocker for this PR, I'm just wondering - and if there's anything we can do now to make our lives easier then it might be worth it.
.circleci/config.yml
exarkun (Migrated from github.com) commented 2020-12-15 12:53:10 +00:00
Copy Link

Like the other images we depend on, we should build this one as part of our CI and push it to the account indicated by DOCKERHUB_CONTEXT.

Also, in a perfect world, this would be a NixOS-based image, and I'd be happy to help with setting it up if you want.

Like the other images we depend on, we should build this one as part of our CI and push it to the account indicated by `DOCKERHUB_CONTEXT`. Also, in a perfect world, this would be a NixOS-based image, and I'd be happy to help with setting it up if you want.
src/allmydata/__init__.py
@ -18,1 +17,4 @@
# type ignored as it fails in CI
# (https://app.circleci.com/pipelines/github/tahoe-lafs/tahoe-lafs/1647/workflows/60ae95d4-abe8-492c-8a03-1ad3b9e42ed3/jobs/40972)
from allmydata._version import __version__ # type: ignore
except ImportError:
exarkun (Migrated from github.com) commented 2020-12-15 13:16:11 +00:00
Copy Link

I deleted this comment and the one below because I was curious what problem mypy reported without them but when I ran mypy it didn't report any problem.

I deleted this comment and the one below because I was curious what problem mypy reported without them but when I ran mypy it didn't report any problem.
src/allmydata/codec.py
@ -58,2 +58,4 @@
return defer.succeed((shares, desired_share_ids))
def encode_proposal(self, data, desired_share_ids=None):
raise NotImplementedError()
exarkun (Migrated from github.com) commented 2020-12-15 13:33:48 +00:00
Copy Link

There seem to be no callers of encode_proposal in the codebase. On the other hand, the method above this one, encode, has the same signature as encode_proposal and seems to have some callers (eg src/allmydata/immutable/encode.py makes a CSREncoder and then calls its encode method. So I think someone forgot the method name between writing the interface and implementing.

Maybe better to rename it on the interface since all of the real implementation thinks it has the other name.

There seem to be no callers of `encode_proposal` in the codebase. On the other hand, the method above this one, `encode`, has the same signature as `encode_proposal` and seems to have some callers (eg `src/allmydata/immutable/encode.py` makes a `CSREncoder` and then calls its `encode` method. So I think someone forgot the method name between writing the interface and implementing. Maybe better to rename it on the interface since all of the real implementation thinks it has the other name.
src/allmydata/frontends/ftpd.py
exarkun (Migrated from github.com) commented 2020-12-15 13:38:29 +00:00
Copy Link

I guess this pattern could instead be precondition(accountfile is None or isinstance(accountfile, unicode), accountfile).

This seems slightly better to me, considering TPTB have decided to eliminate types.NoneType but it's also a larger change from what the code was before so if you don't feel like changing it further that's fine with me too.

I guess this pattern could instead be `precondition(accountfile is None or isinstance(accountfile, unicode), accountfile)`. This seems slightly better to me, considering TPTB have decided to eliminate `types.NoneType` but it's also a larger change from what the code was before so if you don't feel like changing it further that's fine with me too.
src/allmydata/immutable/encode.py
exarkun (Migrated from github.com) commented 2020-12-15 14:02:26 +00:00
Copy Link

I can find no callers of IEncoder.set_size so I think the better option is to delete the method from the interface. Adding force to this argument is that the docstring for IEncoder.set_size claims that it must be called before get_serialized_params which seems to belong to ICodecEncoder (rather than IEncoder) though the docstring doesn't mention this. However, ICodecEncoder has no similar set_size method and gets its size information from __init__ (which is outside of any explicitly declared interface).

I can find no callers of `IEncoder.set_size` so I think the better option is to delete the method from the interface. Adding force to this argument is that the docstring for `IEncoder.set_size` claims that it must be called before `get_serialized_params` which seems to belong to `ICodecEncoder` (rather than `IEncoder`) though the docstring doesn't mention this. However, `ICodecEncoder` has no similar `set_size` method and gets its size information from `__init__` (which is outside of any explicitly declared interface).
src/allmydata/immutable/offloaded.py
exarkun (Migrated from github.com) commented 2020-12-15 14:56:58 +00:00
Copy Link

Can you add a link to https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3561 here?

Can you add a link to https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3561 here?
src/allmydata/immutable/upload.py
exarkun (Migrated from github.com) commented 2020-12-15 14:58:09 +00:00
Copy Link

Neither of these methods has any callers. I think we can delete them from the interface instead.

Neither of these methods has any callers. I think we can delete them from the interface instead.
src/allmydata/introducer/client.py
exarkun (Migrated from github.com) commented 2020-12-15 15:03:48 +00:00
Copy Link

The implementation doesn't actually work with signing_key=None anymore. It probably did long ago. Let's take the default off of the interface (and delete the part of the docstring that claims it is allowed) instead of adding it here.

The implementation doesn't actually work with `signing_key=None` anymore. It probably did long ago. Let's take the default off of the interface (and delete the part of the docstring that claims it is allowed) instead of adding it here.
src/allmydata/mutable/filenode.py
@ -566,3 +566,3 @@
def upload(self, new_contents, servermap):
def upload(self, new_contents, servermap, progress=None):
"""
exarkun (Migrated from github.com) commented 2020-12-15 15:07:20 +00:00
Copy Link

I wonder why mypy doesn't complain about 3eb975748a/src/allmydata/blacklist.py (L152)

I wonder why mypy doesn't complain about https://github.com/tahoe-lafs/tahoe-lafs/blob/3eb975748a9a6c32c7972ce4c498ec5afabfda9d/src/allmydata/blacklist.py#L152
src/allmydata/mutable/filenode.py
exarkun (Migrated from github.com) commented 2020-12-15 15:08:20 +00:00
Copy Link

I deleted this comment and didn't see any new mypy errors.

I deleted this comment and didn't see any new mypy errors.
src/allmydata/mutable/filenode.py
exarkun (Migrated from github.com) commented 2020-12-15 15:16:25 +00:00
Copy Link

I can't find any callers of this get_servermap. Can you link to https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3562 from here?

I can't find any callers of this `get_servermap`. Can you link to https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3562 from here?
src/allmydata/node.py
exarkun (Migrated from github.com) commented 2020-12-15 15:17:08 +00:00
Copy Link

This seems unused, can we get away with just deleting it?

This seems unused, can we get away with just deleting it?
src/allmydata/scripts/common.py
exarkun (Migrated from github.com) commented 2020-12-15 15:24:52 +00:00
Copy Link

Can this be some Parameters typing thing similar to SubCommands?

Can this be some `Parameters` typing thing similar to `SubCommands`?
src/allmydata/scripts/common.py
exarkun (Migrated from github.com) commented 2020-12-15 15:25:36 +00:00
Copy Link

and here

and here
src/allmydata/scripts/runner.py
exarkun (Migrated from github.com) commented 2020-12-15 15:27:58 +00:00
Copy Link

Since these two modules clearly each do have a dispatch attribute I wonder why mypy complains that they don't.

Since these two modules clearly each do have a `dispatch` attribute I wonder why mypy complains that they don't.
src/allmydata/stats.py
exarkun (Migrated from github.com) commented 2020-12-15 15:29:26 +00:00
Copy Link

Wouldn't this be better if it were actually a float instead of an int pretending to be a float? That is, POLL_INTERVAL = 60.0?

Wouldn't this be better if it were actually a float instead of an int pretending to be a float? That is, `POLL_INTERVAL = 60.0`?
src/allmydata/storage/server.py
exarkun (Migrated from github.com) commented 2020-12-15 15:33:18 +00:00
Copy Link

Oh huh. Interesting. I guess this also has to do with foolscap#78? Because it's about mypy thinking that RIStorageServer.slot_testv_and_readv_and_writev is the "supertype" definition of StorageServer.slot_testv_and_readv_and_writev but it's actually the supertype definition of StorageServer.remote_slot_testv_and_readv_and_writev.

Can you link to the foolscap ticket here too, or if you think it's not actually going to be resolved when that ticket is fixed, link to some other ticket describing this particular issue?

Oh huh. Interesting. I guess this also has to do with foolscap#78? Because it's about mypy thinking that `RIStorageServer.slot_testv_and_readv_and_writev` is the "supertype" definition of `StorageServer.slot_testv_and_readv_and_writev` but it's actually the supertype definition of `StorageServer.remote_slot_testv_and_readv_and_writev`. Can you link to the foolscap ticket here too, or if you think it's not actually going to be resolved when that ticket is fixed, link to some other ticket describing this particular issue?
src/allmydata/test/no_network.py
exarkun (Migrated from github.com) commented 2020-12-15 15:37:47 +00:00
Copy Link

File and link to a ticket for writing those stubs?

File and link to a ticket for writing those stubs?
src/allmydata/test/no_network.py
exarkun (Migrated from github.com) commented 2020-12-15 15:39:58 +00:00
Copy Link

That sounds like an awesome one but I can't reproduce it locally.

That sounds like an awesome one but I can't reproduce it locally.
src/allmydata/test/storage_plugin.py
@ -47,8 +47,9 @@ class RIDummy(RemoteInterface):
"""
exarkun (Migrated from github.com) commented 2020-12-15 15:40:48 +00:00
Copy Link

File and link to a ticket for this?

File and link to a ticket for this?
src/allmydata/test/test_dirnode.py
exarkun (Migrated from github.com) commented 2020-12-15 15:51:23 +00:00
Copy Link

rather, incomplete implementation, right?

rather, incomplete implementation, right?
src/allmydata/test/test_dirnode.py
exarkun (Migrated from github.com) commented 2020-12-15 15:51:17 +00:00
Copy Link

Also cannot reproduce this one

Also cannot reproduce this one
src/allmydata/uri.py
exarkun (Migrated from github.com) commented 2020-12-15 17:59:25 +00:00
Copy Link

It looks like all of the subclasses of _DirectoryBaseURI implement these two so I suppose this is fine. I don't know if this is the right path or if it's better to move the implements decoration down to the subclasses.

It looks like all of the subclasses of `_DirectoryBaseURI` implement these two so I suppose this is fine. I don't know if this is the right path or if it's better to move the `implements` decoration down to the subclasses.
src/allmydata/uri.py
exarkun (Migrated from github.com) commented 2020-12-15 18:01:49 +00:00
Copy Link

I'm not sure I know what this means. Are there some mypy or mypy-zope docs I should read? Neither the docstring for typing.Type nor the information on https://realpython.com/python-type-checking/ seems to explain what's happening here.

I'm not sure I know what this means. Are there some mypy or mypy-zope docs I should read? Neither the docstring for `typing.Type` nor the information on https://realpython.com/python-type-checking/ seems to explain what's happening here.
src/allmydata/web/private.py
exarkun (Migrated from github.com) commented 2020-12-15 18:11:34 +00:00
Copy Link

Shoobx/mypy-zope#26 is closed now. It sounds like there is some other resolution required here, perhaps having to do with using some type stubs for Twisted? Alternatively, if you want to move IToken to allmydata.interfaces and that helps then feel free.

Either way, please update the comment to explain the pitfall and link to any outstanding tickets for work that might need to be done to address it.

Shoobx/mypy-zope#26 is closed now. It sounds like there is some other resolution required here, perhaps having to do with using some type stubs for Twisted? Alternatively, if you want to move `IToken` to `allmydata.interfaces` and that helps then feel free. Either way, please update the comment to explain the pitfall and link to any outstanding tickets for work that might need to be done to address it.
src/allmydata/windows/fixups.py
exarkun (Migrated from github.com) commented 2020-12-15 18:14:19 +00:00
Copy Link

maybe mention it is ignored because it doesn't exist on Python 3? I assume this (and other Python 2/3 ignores) will go away as the porting effort reaches this code. It might be nice to help the porter who gets to such code remember to remove the ignore somehow.

maybe mention it is ignored because it doesn't exist on Python 3? I assume this (and other Python 2/3 ignores) will go away as the porting effort reaches this code. It might be nice to help the porter who gets to such code remember to remove the ignore somehow.
tox.ini
exarkun (Migrated from github.com) commented 2020-12-15 13:10:06 +00:00
Copy Link

I think I'd like to see both of these merged to their respective masters before we merge this PR, fwiw.

I think I'd like to see both of these merged to their respective masters before we merge this PR, fwiw.
jaraco (Migrated from github.com) reviewed 2020-12-18 14:06:27 +00:00
.circleci/config.yml
jaraco (Migrated from github.com) commented 2020-12-18 14:06:26 +00:00
Copy Link

Agreed. There's nothing special about this image except that it has tox on a late Python 3. Probably the existing images can work - they might need some tweaks to support this. I was hoping someone familiar with those images could figure out the recipe after I had something stable on jaraco/multipy-tox, so yes, I'd appreciate the help.

Agreed. There's nothing special about this image except that it has tox on a late Python 3. Probably the existing images can work - they might need some tweaks to support this. I was hoping someone familiar with those images could figure out the recipe after I had something stable on jaraco/multipy-tox, so yes, I'd appreciate the help.
jaraco (Migrated from github.com) reviewed 2020-12-18 14:08:55 +00:00
tox.ini
jaraco (Migrated from github.com) commented 2020-12-18 14:08:54 +00:00
Copy Link

I definitely share that preference. I saw it as a non-blocker, as the checks can have their benefit without the patches, but I would definitely like to see these merged, and making it a blocker will incentivize getting those merged and released.

I definitely share that preference. I saw it as a non-blocker, as the checks can have their benefit without the patches, but I would definitely like to see these merged, and making it a blocker will incentivize getting those merged and released.
jaraco (Migrated from github.com) reviewed 2020-12-18 14:24:23 +00:00
src/allmydata/__init__.py
@ -18,1 +17,4 @@
# type ignored as it fails in CI
# (https://app.circleci.com/pipelines/github/tahoe-lafs/tahoe-lafs/1647/workflows/60ae95d4-abe8-492c-8a03-1ad3b9e42ed3/jobs/40972)
from allmydata._version import __version__ # type: ignore
except ImportError:
jaraco (Migrated from github.com) commented 2020-12-18 14:24:23 +00:00
Copy Link

Hmm. Perhaps mypy improved these cases. I searched and didn't find anything about it. I tried and can't find a way to replicate whatever was happening, so I'll revert this change.

Hmm. Perhaps mypy improved these cases. I searched and didn't find anything about it. I tried and can't find a way to replicate whatever was happening, so I'll revert this change.
jaraco (Migrated from github.com) reviewed 2020-12-18 14:35:43 +00:00
src/allmydata/__init__.py
@ -18,1 +17,4 @@
# type ignored as it fails in CI
# (https://app.circleci.com/pipelines/github/tahoe-lafs/tahoe-lafs/1647/workflows/60ae95d4-abe8-492c-8a03-1ad3b9e42ed3/jobs/40972)
from allmydata._version import __version__ # type: ignore
except ImportError:
jaraco (Migrated from github.com) commented 2020-12-18 14:35:42 +00:00
Copy Link

Removed in 950ca1893

Removed in 950ca1893
jaraco (Migrated from github.com) reviewed 2020-12-18 14:53:33 +00:00
src/allmydata/frontends/ftpd.py
jaraco (Migrated from github.com) commented 2020-12-18 14:53:33 +00:00
Copy Link

I definitely aim to minimize the impact on behavior, so I'll stick with this change for now, but I agree it may be preferable to use the idiomatic form separately.

I definitely aim to minimize the impact on behavior, so I'll stick with this change for now, but I agree it may be preferable to use the idiomatic form separately.
jaraco (Migrated from github.com) reviewed 2020-12-18 14:59:29 +00:00
src/allmydata/test/test_dirnode.py
jaraco (Migrated from github.com) commented 2020-12-18 14:59:29 +00:00
Copy Link

Right. I think what I was trying to say (succinctly enough to fit within the line length limit) was that the interface of this class is incomplete with respect to its declared interface. Fixed in 99da74fff.

Right. I think what I was trying to say (succinctly enough to fit within the line length limit) was that the interface of this class is incomplete with respect to its declared interface. Fixed in 99da74fff.
jaraco (Migrated from github.com) reviewed 2020-12-18 15:06:16 +00:00
src/allmydata/uri.py
jaraco (Migrated from github.com) commented 2020-12-18 15:06:16 +00:00
Copy Link

Indeed. I don't know either. In other cases, I went the other route. It's a little annoying that every subclass would need to have redundant declarations (interface + subclass), but that's probably a better approach than implementing/not-implementing the interface in the parent class (as seen here).

Indeed. I don't know either. In other cases, I went the other route. It's a little annoying that every subclass would need to have redundant declarations (interface + subclass), but that's probably a better approach than implementing/not-implementing the interface in the parent class (as seen here).
jaraco (Migrated from github.com) reviewed 2020-12-18 15:22:34 +00:00
src/allmydata/uri.py
jaraco (Migrated from github.com) commented 2020-12-18 15:22:34 +00:00
Copy Link

I don't have any good references. I've just been picking it up by trial-and-error. What I think this is saying is the INNER_URI_CLASS is a Type (aka class) that's an instance of the metaclass IVerifierURI. That's only really a guess, though. I admit, this approach may not be correct, but it's one solution that unblocks the errors that occur when subclasses of DirectoryURIVerifier indicate a different type for INNER_URI_CLASS. The intention here is to declare some type that's common among all DirectoryURIVerifiers.

I don't have any good references. I've just been picking it up by trial-and-error. What I think this is saying is the `INNER_URI_CLASS` is a `Type` (aka class) that's an instance of the metaclass `IVerifierURI`. That's only really a guess, though. I admit, this approach may not be correct, but it's one solution that unblocks the errors that occur when subclasses of DirectoryURIVerifier indicate a different type for INNER_URI_CLASS. The intention here is to declare some type that's common among all DirectoryURIVerifiers.
jaraco (Migrated from github.com) reviewed 2020-12-18 15:35:23 +00:00
src/allmydata/web/private.py
jaraco (Migrated from github.com) commented 2020-12-18 15:35:22 +00:00
Copy Link

That's right - having type stubs for Twisted is the proper fix. Clarified in 51b0b201b.

That's right - having type stubs for Twisted is the proper fix. Clarified in 51b0b201b.
jaraco (Migrated from github.com) reviewed 2020-12-18 15:36:59 +00:00
src/allmydata/windows/fixups.py
jaraco (Migrated from github.com) commented 2020-12-18 15:36:59 +00:00
Copy Link

Fixed in efd0aef28.

Fixed in efd0aef28.
jaraco (Migrated from github.com) reviewed 2020-12-18 15:48:59 +00:00
src/allmydata/codec.py
@ -58,2 +58,4 @@
return defer.succeed((shares, desired_share_ids))
def encode_proposal(self, data, desired_share_ids=None):
raise NotImplementedError()
jaraco (Migrated from github.com) commented 2020-12-18 15:48:59 +00:00
Copy Link

It looks like encode_proposal is a proposed but not-yet-implemented replacement for encode. See the docstrings in interfaces.py, especially these:
d19b1cfd68/src/allmydata/interfaces.py (L1706-L1716)
d19b1cfd68/src/allmydata/interfaces.py (L1724-L1726)

Renaming the method name in the interface isn't possible as the interface definition for encode also exists and provides a different interface.

I believe this implementation as proposed is the cleanest given the proposal has apparently not been accepted. I would recommend removing this proposed interface, replacing it with an issue or PR to capture the work in progress, but independent of this PR.

It looks like `encode_proposal` is a proposed but not-yet-implemented replacement for `encode`. See the docstrings in interfaces.py, especially these: https://github.com/tahoe-lafs/tahoe-lafs/blob/d19b1cfd687cfede9374d994aca7c1890af6811e/src/allmydata/interfaces.py#L1706-L1716 https://github.com/tahoe-lafs/tahoe-lafs/blob/d19b1cfd687cfede9374d994aca7c1890af6811e/src/allmydata/interfaces.py#L1724-L1726 Renaming the method name in the interface isn't possible as the interface definition for encode also exists and provides a different interface. I believe this implementation as proposed is the cleanest given the proposal has apparently not been accepted. I would recommend removing this proposed interface, replacing it with an issue or PR to capture the work in progress, but independent of this PR.
jaraco (Migrated from github.com) reviewed 2020-12-18 15:52:09 +00:00
src/allmydata/immutable/encode.py
jaraco (Migrated from github.com) commented 2020-12-18 15:52:09 +00:00
Copy Link

Fixed in ea0c10ef8.

Fixed in ea0c10ef8.
jaraco (Migrated from github.com) reviewed 2020-12-18 15:53:41 +00:00
src/allmydata/immutable/offloaded.py
jaraco (Migrated from github.com) commented 2020-12-18 15:53:41 +00:00
Copy Link

Fixed in d051791e9.

Fixed in d051791e9.
jaraco (Migrated from github.com) reviewed 2020-12-18 15:55:49 +00:00
src/allmydata/immutable/upload.py
jaraco (Migrated from github.com) commented 2020-12-18 15:55:49 +00:00
Copy Link

Fixed in 090031cbf.

Fixed in 090031cbf.
jaraco (Migrated from github.com) reviewed 2020-12-18 16:00:07 +00:00
src/allmydata/introducer/client.py
jaraco (Migrated from github.com) commented 2020-12-18 16:00:06 +00:00
Copy Link

Fixed in 0e248cb4e.

Fixed in 0e248cb4e.
jaraco (Migrated from github.com) reviewed 2020-12-18 16:02:36 +00:00
src/allmydata/mutable/filenode.py
jaraco (Migrated from github.com) commented 2020-12-18 16:02:36 +00:00
Copy Link

Here's what I see:

tahoe-lafs 3399.mypy $ git diff
diff --git a/src/allmydata/mutable/filenode.py b/src/allmydata/mutable/filenode.py
index 4613a918b..e9cf23fa1 100644
--- a/src/allmydata/mutable/filenode.py
+++ b/src/allmydata/mutable/filenode.py
@@ -951,7 +951,7 @@ class MutableFileVersion(object):
         return self._servermap.size_of_version(self._version)
 
 
-    def download_to_data(self, fetch_privkey=False, progress=None):  # type: ignore # fixme
+    def download_to_data(self, fetch_privkey=False, progress=None):
         """
         I return a Deferred that fires with the contents of this
         readable object as a byte string.

tahoe-lafs 3399.mypy $ tox -e typechecks
typechecks installed: attrs==20.3.0,Automat==20.2.0,cffi==1.14.4,constantly==15.1.0,cryptography==3.3.1,foolscap @ git+https://github.com/jaraco/foolscap@d70d8ecee926d3693b9fac55aebe13573d4f71c9,hyperlink==20.0.1,idna==2.10,incremental==17.5.0,mypy==0.790,mypy-extensions==0.4.3,mypy-zope @ git+https://github.com/jaraco/mypy-zope@48f912cc3cdb4ee37d4d3783ef466a7dce8bf0a9,pyasn1==0.4.8,pyasn1-modules==0.2.8,pycparser==2.20,PyHamcrest==2.0.2,pyOpenSSL==20.0.1,service-identity==18.1.0,six==1.15.0,Twisted==20.3.0,typed-ast==1.4.1,typing-extensions==3.7.4.3,zope.event==4.5.0,zope.interface==5.2.0,zope.schema==6.0.0
typechecks run-test-pre: PYTHONHASHSEED='1134400167'
typechecks run-test: commands[0] | mypy src
src/allmydata/mutable/filenode.py:954: error: Signature of "MutableFileVersion" incompatible with "download_to_data" of supertype "allmydata.interfaces.IReadable"
Found 1 error in 1 file (checked 293 source files)
ERROR: InvocationError for command /Users/jaraco/code/public/tahoe-lafs/.tox/typechecks/bin/mypy src (exited with code 1)
________________________________________________________________ summary ________________________________________________________________
ERROR:   typechecks: commands failed
Here's what I see: ``` tahoe-lafs 3399.mypy $ git diff diff --git a/src/allmydata/mutable/filenode.py b/src/allmydata/mutable/filenode.py index 4613a918b..e9cf23fa1 100644 --- a/src/allmydata/mutable/filenode.py +++ b/src/allmydata/mutable/filenode.py @@ -951,7 +951,7 @@ class MutableFileVersion(object): return self._servermap.size_of_version(self._version) - def download_to_data(self, fetch_privkey=False, progress=None): # type: ignore # fixme + def download_to_data(self, fetch_privkey=False, progress=None): """ I return a Deferred that fires with the contents of this readable object as a byte string. tahoe-lafs 3399.mypy $ tox -e typechecks typechecks installed: attrs==20.3.0,Automat==20.2.0,cffi==1.14.4,constantly==15.1.0,cryptography==3.3.1,foolscap @ git+https://github.com/jaraco/foolscap@d70d8ecee926d3693b9fac55aebe13573d4f71c9,hyperlink==20.0.1,idna==2.10,incremental==17.5.0,mypy==0.790,mypy-extensions==0.4.3,mypy-zope @ git+https://github.com/jaraco/mypy-zope@48f912cc3cdb4ee37d4d3783ef466a7dce8bf0a9,pyasn1==0.4.8,pyasn1-modules==0.2.8,pycparser==2.20,PyHamcrest==2.0.2,pyOpenSSL==20.0.1,service-identity==18.1.0,six==1.15.0,Twisted==20.3.0,typed-ast==1.4.1,typing-extensions==3.7.4.3,zope.event==4.5.0,zope.interface==5.2.0,zope.schema==6.0.0 typechecks run-test-pre: PYTHONHASHSEED='1134400167' typechecks run-test: commands[0] | mypy src src/allmydata/mutable/filenode.py:954: error: Signature of "MutableFileVersion" incompatible with "download_to_data" of supertype "allmydata.interfaces.IReadable" Found 1 error in 1 file (checked 293 source files) ERROR: InvocationError for command /Users/jaraco/code/public/tahoe-lafs/.tox/typechecks/bin/mypy src (exited with code 1) ________________________________________________________________ summary ________________________________________________________________ ERROR: typechecks: commands failed ```
jaraco (Migrated from github.com) reviewed 2020-12-18 16:06:11 +00:00
src/allmydata/mutable/filenode.py
jaraco (Migrated from github.com) commented 2020-12-18 16:06:11 +00:00
Copy Link

Fixed in c2d2aba83.

Fixed in c2d2aba83.
jaraco (Migrated from github.com) reviewed 2020-12-18 16:08:24 +00:00
src/allmydata/node.py
jaraco (Migrated from github.com) commented 2020-12-18 16:08:24 +00:00
Copy Link

Sure. I'd observed the same, but gave the code the benefit of the doubt and left it around in case it was used through some mysterious usage I couldn't detect. Fixed in 189608e11.

Sure. I'd observed the same, but gave the code the benefit of the doubt and left it around in case it was used through some mysterious usage I couldn't detect. Fixed in 189608e11.
jaraco (Migrated from github.com) reviewed 2020-12-18 16:14:29 +00:00
src/allmydata/scripts/common.py
jaraco (Migrated from github.com) commented 2020-12-18 16:14:28 +00:00
Copy Link

Great idea. Fixed in 602a06e5c.

Great idea. Fixed in 602a06e5c.
jaraco (Migrated from github.com) reviewed 2020-12-18 16:14:37 +00:00
src/allmydata/scripts/common.py
jaraco (Migrated from github.com) commented 2020-12-18 16:14:37 +00:00
Copy Link

602a06e5c

602a06e5c
jaraco (Migrated from github.com) reviewed 2020-12-18 16:26:52 +00:00
src/allmydata/scripts/runner.py
jaraco (Migrated from github.com) commented 2020-12-18 16:26:52 +00:00
Copy Link

I wondered the same thing. I suspect it's because (create_node, stats_gatherer) resolves to Tuple[ModuleType] where ModuleType is the common interface of the create_node and stats_gatherer modules and ModuleType doesn't generally have a dispatch attribute. My guess is one would need to create an interface. to satisfy this check, and that's not simple because it would also require creating a type for the dispatch property as well.

I wondered the same thing. I suspect it's because `(create_node, stats_gatherer)` resolves to `Tuple[ModuleType]` where `ModuleType` is the common interface of the `create_node` and `stats_gatherer` modules and `ModuleType` doesn't generally have a `dispatch` attribute. My guess is one would need to create an interface. to satisfy this check, and that's not simple because it would also require creating a type for the dispatch property as well.
jaraco (Migrated from github.com) reviewed 2020-12-18 16:28:54 +00:00
src/allmydata/stats.py
jaraco (Migrated from github.com) commented 2020-12-18 16:28:54 +00:00
Copy Link

This was surprising to me too. I learned that int is a subtype of float, so if you want to declare a number that can be an integer or float, you just declare it as a float. I would recommend against adding .0 or .00. Integers are acceptable here and should be used where convenient and appropriate.

This was surprising to me too. I learned that `int` is a subtype of `float`, so if you want to declare a number that can be an integer or float, you just declare it as a float. I would recommend against adding `.0` or `.00`. Integers are acceptable here and should be used where convenient and appropriate.
jaraco commented 2020-12-18 16:30:32 +00:00 (Migrated from github.com)
Copy Link

Thanks for the insightful review. I've addressed some of the concerns but have many more to go. I'm taking a break and will resume later.

Thanks for the insightful review. I've addressed some of the concerns but have many more to go. I'm taking a break and will resume later.
jaraco (Migrated from github.com) reviewed 2020-12-26 15:44:41 +00:00
tox.ini
jaraco (Migrated from github.com) commented 2020-12-26 15:44:41 +00:00
Copy Link

Pull requests:

Pull requests: - [x] warner/foolscap#74 - [x] warner/foolscap#76 - [x] Shoobx/mypy-zope#24
jaraco (Migrated from github.com) reviewed 2020-12-26 16:17:53 +00:00
src/allmydata/uri.py
jaraco (Migrated from github.com) commented 2020-12-26 16:17:53 +00:00
Copy Link

Fixed in 6b6b8f837. PTAL.

Fixed in 6b6b8f837. PTAL.
jaraco (Migrated from github.com) reviewed 2020-12-26 16:24:47 +00:00
src/allmydata/__init__.py
@ -18,1 +17,4 @@
# type ignored as it fails in CI
# (https://app.circleci.com/pipelines/github/tahoe-lafs/tahoe-lafs/1647/workflows/60ae95d4-abe8-492c-8a03-1ad3b9e42ed3/jobs/40972)
from allmydata._version import __version__ # type: ignore
except ImportError:
jaraco (Migrated from github.com) commented 2020-12-26 16:24:47 +00:00
Copy Link

Turns out the tests fail in CI but not locally. Not sure why the difference. Regardless, I'll add a comment.

Turns out the [tests fail in CI](https://app.circleci.com/pipelines/github/tahoe-lafs/tahoe-lafs/1647/workflows/60ae95d4-abe8-492c-8a03-1ad3b9e42ed3/jobs/40972) but not locally. Not sure why the difference. Regardless, I'll add a comment.
jaraco (Migrated from github.com) reviewed 2020-12-26 16:26:08 +00:00
src/allmydata/__init__.py
@ -18,1 +17,4 @@
# type ignored as it fails in CI
# (https://app.circleci.com/pipelines/github/tahoe-lafs/tahoe-lafs/1647/workflows/60ae95d4-abe8-492c-8a03-1ad3b9e42ed3/jobs/40972)
from allmydata._version import __version__ # type: ignore
except ImportError:
jaraco (Migrated from github.com) commented 2020-12-26 16:26:07 +00:00
Copy Link

Brought back in b65ef3cee with a comment showing where it fails.

Brought back in b65ef3cee with a comment showing where it fails.
jaraco (Migrated from github.com) reviewed 2020-12-26 16:52:29 +00:00
src/allmydata/uri.py
jaraco (Migrated from github.com) commented 2020-12-26 16:52:29 +00:00
Copy Link

That commit was insufficient. Amended with ab2c544efc.

That commit was insufficient. Amended with ab2c544efcaad629f2d49173bfdc82e8de94d944.
jaraco (Migrated from github.com) reviewed 2020-12-26 16:57:31 +00:00
src/allmydata/test/storage_plugin.py
@ -47,8 +47,9 @@ class RIDummy(RemoteInterface):
"""
jaraco (Migrated from github.com) commented 2020-12-26 16:57:31 +00:00
Copy Link

In dacdf7f12, I added a link to the existing upstream issue.

In dacdf7f12, I added a link to the existing upstream issue.
jaraco (Migrated from github.com) reviewed 2020-12-26 17:03:39 +00:00
src/allmydata/storage/server.py
jaraco (Migrated from github.com) commented 2020-12-26 17:03:38 +00:00
Copy Link

I agree, it probably does have to do with that issue. I think I'd missed that detail. Fixed in 5396f9f97.

I agree, it probably does have to do with that issue. I think I'd missed that detail. Fixed in 5396f9f97.
jaraco (Migrated from github.com) reviewed 2020-12-26 17:13:01 +00:00
src/allmydata/test/no_network.py
jaraco (Migrated from github.com) commented 2020-12-26 17:13:01 +00:00
Copy Link

Done in 1bf71fd69.

Done in 1bf71fd69.
jaraco (Migrated from github.com) reviewed 2020-12-26 17:24:39 +00:00
src/allmydata/test/no_network.py
jaraco (Migrated from github.com) commented 2020-12-26 17:24:39 +00:00
Copy Link

Here's what I see:

tahoe-lafs 3399.mypy $ git diff
diff --git a/src/allmydata/test/no_network.py b/src/allmydata/test/no_network.py
index ba497f81a..17a63a3a2 100644
--- a/src/allmydata/test/no_network.py
+++ b/src/allmydata/test/no_network.py
@@ -261,7 +261,7 @@ def create_no_network_client(basedir):
     return defer.succeed(client)
 
 
-class _NoNetworkClient(_Client):  # type: ignore # Cannot determine consistent MRO order
+class _NoNetworkClient(_Client):
     """
     Overrides all _Client networking functionality to do nothing.
     """

tahoe-lafs 3399.mypy $ tox -e typechecks
typechecks installed: attrs==20.3.0,Automat==20.2.0,cffi==1.14.4,constantly==15.1.0,cryptography==3.3.1,foolscap @ git+https://github.com/jaraco/foolscap@d70d8ecee926d3693b9fac55aebe13573d4f71c9,hyperlink==20.0.1,idna==2.10,incremental==17.5.0,mypy==0.790,mypy-extensions==0.4.3,mypy-zope @ git+https://github.com/jaraco/mypy-zope@48f912cc3cdb4ee37d4d3783ef466a7dce8bf0a9,pyasn1==0.4.8,pyasn1-modules==0.2.8,pycparser==2.20,PyHamcrest==2.0.2,pyOpenSSL==20.0.1,service-identity==18.1.0,six==1.15.0,Twisted==20.3.0,typed-ast==1.4.1,typing-extensions==3.7.4.3,zope.event==4.5.0,zope.interface==5.2.0,zope.schema==6.0.0
typechecks run-test-pre: PYTHONHASHSEED='3588652114'
typechecks run-test: commands[0] | mypy src
src/allmydata/test/no_network.py:264: error: Cannot determine consistent method resolution order (MRO) for "_NoNetworkClient"
Found 1 error in 1 file (checked 293 source files)
ERROR: InvocationError for command /Users/jaraco/code/public/tahoe-lafs/.tox/typechecks/bin/mypy src (exited with code 1)
_______________________________________________________________ summary _______________________________________________________________
ERROR:   typechecks: commands failed

I don't understand why the issue exists on some platforms but not others. Probably we need an issue to track this, since it's not consistent.

Here's what I see: ``` tahoe-lafs 3399.mypy $ git diff diff --git a/src/allmydata/test/no_network.py b/src/allmydata/test/no_network.py index ba497f81a..17a63a3a2 100644 --- a/src/allmydata/test/no_network.py +++ b/src/allmydata/test/no_network.py @@ -261,7 +261,7 @@ def create_no_network_client(basedir): return defer.succeed(client) -class _NoNetworkClient(_Client): # type: ignore # Cannot determine consistent MRO order +class _NoNetworkClient(_Client): """ Overrides all _Client networking functionality to do nothing. """ tahoe-lafs 3399.mypy $ tox -e typechecks typechecks installed: attrs==20.3.0,Automat==20.2.0,cffi==1.14.4,constantly==15.1.0,cryptography==3.3.1,foolscap @ git+https://github.com/jaraco/foolscap@d70d8ecee926d3693b9fac55aebe13573d4f71c9,hyperlink==20.0.1,idna==2.10,incremental==17.5.0,mypy==0.790,mypy-extensions==0.4.3,mypy-zope @ git+https://github.com/jaraco/mypy-zope@48f912cc3cdb4ee37d4d3783ef466a7dce8bf0a9,pyasn1==0.4.8,pyasn1-modules==0.2.8,pycparser==2.20,PyHamcrest==2.0.2,pyOpenSSL==20.0.1,service-identity==18.1.0,six==1.15.0,Twisted==20.3.0,typed-ast==1.4.1,typing-extensions==3.7.4.3,zope.event==4.5.0,zope.interface==5.2.0,zope.schema==6.0.0 typechecks run-test-pre: PYTHONHASHSEED='3588652114' typechecks run-test: commands[0] | mypy src src/allmydata/test/no_network.py:264: error: Cannot determine consistent method resolution order (MRO) for "_NoNetworkClient" Found 1 error in 1 file (checked 293 source files) ERROR: InvocationError for command /Users/jaraco/code/public/tahoe-lafs/.tox/typechecks/bin/mypy src (exited with code 1) _______________________________________________________________ summary _______________________________________________________________ ERROR: typechecks: commands failed ``` I don't understand why the issue exists on some platforms but not others. Probably we need an issue to track this, since it's not consistent.
jaraco (Migrated from github.com) reviewed 2020-12-26 17:33:40 +00:00
src/allmydata/test/no_network.py
jaraco (Migrated from github.com) commented 2020-12-26 17:33:39 +00:00
Copy Link

Addressed in 01147f462.

Addressed in 01147f462.
jaraco (Migrated from github.com) reviewed 2020-12-26 17:33:51 +00:00
src/allmydata/test/test_dirnode.py
jaraco (Migrated from github.com) commented 2020-12-26 17:33:51 +00:00
Copy Link

Addressed in 01147f462.

Addressed in 01147f462.
jaraco (Migrated from github.com) reviewed 2020-12-26 17:38:42 +00:00
src/allmydata/mutable/filenode.py
@ -566,3 +566,3 @@
def upload(self, new_contents, servermap):
def upload(self, new_contents, servermap, progress=None):
"""
jaraco (Migrated from github.com) commented 2020-12-26 17:38:42 +00:00
Copy Link

blacklist.ProhibitedNode implements IFileNode, which doesn't specify an upload method. On the other hand, mutable.filenode.MutableFileNode implements IMutableFileNode, which does stipulate the interface.

`blacklist.ProhibitedNode` implements `IFileNode`, which doesn't specify an `upload` method. On the other hand, `mutable.filenode.MutableFileNode` implements `IMutableFileNode`, which does stipulate the interface.
jaraco commented 2020-12-26 17:49:30 +00:00 (Migrated from github.com)
Copy Link

In 574613a89, I've merged with master and resolved the conflicts. Doing so revealed 8 new errors:

src/allmydata/node.py:302: error: Need type annotation for '_basedir'
src/allmydata/node.py:303: error: Unsupported converter, only named functions and types are currently supported
src/allmydata/test/test_node.py:567: error: "Callable[[TestMissingPorts], Any]" has no attribute "todo"
src/allmydata/test/test_introducer.py:952: error: Dict entry 0 has incompatible type "str": "Dict[int, <nothing>]"; expected "bytes": "Collection[int]"
src/allmydata/test/test_introducer.py:954: error: Dict entry 1 has incompatible type "str": "str"; expected "bytes": "Collection[int]"
src/allmydata/test/web/test_root.py:3: error: Module 'urllib' has no attribute 'quote'
src/allmydata/test/test_storage_client.py:573: error: zope.interface.implementer accepts interface, not foolscap.ipb.IConnectionHintHandler.
src/allmydata/test/test_storage_client.py:573: error: Make sure you have stubs for all packages that provide interfaces for foolscap.ipb.IConnectionHintHandler class hierarchy.
In 574613a89, I've merged with master and resolved the conflicts. Doing so revealed 8 new errors: ``` src/allmydata/node.py:302: error: Need type annotation for '_basedir' src/allmydata/node.py:303: error: Unsupported converter, only named functions and types are currently supported src/allmydata/test/test_node.py:567: error: "Callable[[TestMissingPorts], Any]" has no attribute "todo" src/allmydata/test/test_introducer.py:952: error: Dict entry 0 has incompatible type "str": "Dict[int, <nothing>]"; expected "bytes": "Collection[int]" src/allmydata/test/test_introducer.py:954: error: Dict entry 1 has incompatible type "str": "str"; expected "bytes": "Collection[int]" src/allmydata/test/web/test_root.py:3: error: Module 'urllib' has no attribute 'quote' src/allmydata/test/test_storage_client.py:573: error: zope.interface.implementer accepts interface, not foolscap.ipb.IConnectionHintHandler. src/allmydata/test/test_storage_client.py:573: error: Make sure you have stubs for all packages that provide interfaces for foolscap.ipb.IConnectionHintHandler class hierarchy. ```
jaraco commented 2020-12-26 18:13:47 +00:00 (Migrated from github.com)
Copy Link

[Resolving conflicts] revealed 8 new errors

The five commits through 652222116 address those errors.

> [Resolving conflicts] revealed 8 new errors The five commits through 652222116 address those errors.
jaraco commented 2020-12-26 18:35:39 +00:00 (Migrated from github.com)
Copy Link
  • There are some conflicts with master now, alas.

This is addressed (for now).

  • CI isn't completely happy.

With 4a9d3bde5, I believe this is close, if not ready.

  • Are we going to need a ratchet-style job for mypy checks at some point? It seems like at the end of this PR we have mypy running with no errors but, eg, are a ton of errors going to appear when we bring in the type stubs for our dependencies? Since that's later it doesn't need to be a blocker for this PR, I'm just wondering - and if there's anything we can do now to make our lives easier then it might be worth it.

I'm not familiar with "ratchet-style job", but my expectation is that there may be some incremental improvements that over time require some maintenance, but overall they should be small. You may be right about there being some maintenance required if foolscap or twisted were to supply stubs, but my instinct is that we can pin on old versions or otherwise degrade the stubs on those projects to maintain status quo if needed.

I believe I've largely if not fully addressed all of the concerns above to the extent that I can (notably, merging PRs in foolscap and mypy-zope are out of my control). Please take another look.

> * There are some conflicts with master now, alas. This is addressed (for now). > * CI isn't completely happy. With 4a9d3bde5, I believe this is close, if not ready. > * Are we going to need a ratchet-style job for mypy checks at some point? It seems like at the end of _this_ PR we have mypy running with no errors but, eg, are a ton of errors going to appear when we bring in the type stubs for our dependencies? Since that's later it doesn't need to be a blocker for this PR, I'm just wondering - and if there's anything we can do now to make our lives easier then it might be worth it. I'm not familiar with "ratchet-style job", but my expectation is that there _may_ be some incremental improvements that over time require some maintenance, but overall they should be small. You may be right about there being some maintenance required if foolscap or twisted were to supply stubs, but my instinct is that we can pin on old versions or otherwise degrade the stubs on those projects to maintain status quo if needed. --- I believe I've largely if not fully addressed all of the concerns above to the extent that I can (notably, merging PRs in foolscap and mypy-zope are out of my control). Please take another look.
codecov[bot] commented 2020-12-26 19:02:20 +00:00 (Migrated from github.com)
Copy Link

Codecov Report

Merging #915 (c7a4cdb) into master (1ad9e36) will decrease coverage by 0%.
The diff coverage is 74%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #915   +/-   ##
======================================
- Coverage      92%     92%   -0%     
======================================
  Files         150     151    +1     
  Lines       26938   27000   +62     
  Branches     4042    4042           
======================================
+ Hits        24895   24931   +36     
- Misses       1401    1427   +26     
  Partials      642     642
Impacted Files Coverage Δ
src/allmydata/windows/fixups.py 37% <0%> (ø)
src/allmydata/codec.py 97% <50%> (-2%) ⬇️
src/allmydata/scripts/admin.py 73% <50%> (-2%) ⬇️
src/allmydata/scripts/cli.py 97% <50%> (-1%) ⬇️
src/allmydata/scripts/create_node.py 97% <50%> (-1%) ⬇️
src/allmydata/scripts/debug.py 86% <50%> (-<1%) ⬇️
src/allmydata/scripts/tahoe_invite.py 94% <50%> (-3%) ⬇️
src/allmydata/introducer/server.py 95% <60%> (-1%) ⬇️
src/allmydata/scripts/runner.py 73% <60%> (-1%) ⬇️
src/allmydata/util/pollmixin.py 81% <60%> (-3%) ⬇️
... and 22 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 1ad9e36...c7a4cdb. Read the comment docs.

# [Codecov](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915?src=pr&el=h1) Report > Merging [#915](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915?src=pr&el=desc) (c7a4cdb) into [master](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/commit/1ad9e365c34f84265a8dcff0ac82f01b573c890b?el=desc) (1ad9e36) will **decrease** coverage by `0%`. > The diff coverage is `74%`. [![Impacted file tree graph](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/graphs/tree.svg?width=650&height=150&src=pr&token=Ztmu9sr4vi)](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #915 +/- ## ====================================== - Coverage 92% 92% -0% ====================================== Files 150 151 +1 Lines 26938 27000 +62 Branches 4042 4042 ====================================== + Hits 24895 24931 +36 - Misses 1401 1427 +26 Partials 642 642 ``` | [Impacted Files](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [src/allmydata/windows/fixups.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS93aW5kb3dzL2ZpeHVwcy5weQ==) | `37% <0%> (ø)` | | | [src/allmydata/codec.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9jb2RlYy5weQ==) | `97% <50%> (-2%)` | :arrow_down: | | [src/allmydata/scripts/admin.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9zY3JpcHRzL2FkbWluLnB5) | `73% <50%> (-2%)` | :arrow_down: | | [src/allmydata/scripts/cli.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9zY3JpcHRzL2NsaS5weQ==) | `97% <50%> (-1%)` | :arrow_down: | | [src/allmydata/scripts/create\_node.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9zY3JpcHRzL2NyZWF0ZV9ub2RlLnB5) | `97% <50%> (-1%)` | :arrow_down: | | [src/allmydata/scripts/debug.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9zY3JpcHRzL2RlYnVnLnB5) | `86% <50%> (-<1%)` | :arrow_down: | | [src/allmydata/scripts/tahoe\_invite.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9zY3JpcHRzL3RhaG9lX2ludml0ZS5weQ==) | `94% <50%> (-3%)` | :arrow_down: | | [src/allmydata/introducer/server.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9pbnRyb2R1Y2VyL3NlcnZlci5weQ==) | `95% <60%> (-1%)` | :arrow_down: | | [src/allmydata/scripts/runner.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS9zY3JpcHRzL3J1bm5lci5weQ==) | `73% <60%> (-1%)` | :arrow_down: | | [src/allmydata/util/pollmixin.py](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree#diff-c3JjL2FsbG15ZGF0YS91dGlsL3BvbGxtaXhpbi5weQ==) | `81% <60%> (-3%)` | :arrow_down: | | ... and [22 more](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915/diff?src=pr&el=tree-more) | | ------ [Continue to review full report at Codecov](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915?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/915?src=pr&el=footer). Last update [1ad9e36...c7a4cdb](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/915?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
exarkun (Migrated from github.com) reviewed 2021-01-15 20:06:08 +00:00
.circleci/config.yml
exarkun (Migrated from github.com) commented 2021-01-15 20:06:08 +00:00
Copy Link

Pushed a change that switches this to one of our Ubuntu images that has Python 3 and tox. Here's an example CI run:

https://app.circleci.com/pipelines/github/LeastAuthority/tahoe-lafs/154/workflows/2dd439d9-514a-4cd3-bc8d-2ccbe6ea8cb9/jobs/2485

Pushed a change that switches this to one of our Ubuntu images that has Python 3 and tox. Here's an example CI run: https://app.circleci.com/pipelines/github/LeastAuthority/tahoe-lafs/154/workflows/2dd439d9-514a-4cd3-bc8d-2ccbe6ea8cb9/jobs/2485
jaraco (Migrated from github.com) reviewed 2021-01-15 20:28:02 +00:00
.circleci/config.yml
jaraco (Migrated from github.com) commented 2021-01-15 20:28:01 +00:00
Copy Link

Change is a01078ddec. Looks good! Thanks for doing that.

Change is a01078ddec86e1d193c7940087d6506d376028d8. Looks good! Thanks for doing that.
jaraco (Migrated from github.com) reviewed 2021-01-15 20:32:43 +00:00
tox.ini
jaraco (Migrated from github.com) commented 2021-01-15 20:32:43 +00:00
Copy Link

Now that those are merged and c7a4cdb44d is committed, the project now relies on the main branch of the canonical repo.

Now that those are merged and c7a4cdb44d801034de1e068f10aa3d8df8a4950f is committed, the project now relies on the main branch of the canonical repo.
jaraco commented 2021-01-15 20:34:03 +00:00 (Migrated from github.com)
Copy Link

Now that the blockers are cleared, I recommend to merge this change, and defer any emergent loose ends to follow-up efforts.

Now that the blockers are cleared, I recommend to merge this change, and defer any emergent loose ends to follow-up efforts.
exarkun (Migrated from github.com) approved these changes 2021-01-15 21:00:38 +00:00
exarkun (Migrated from github.com) left a comment
Copy Link

Thanks! Looks good. Merging.

Thanks! Looks good. Merging.
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#915
No description provided.
Delete Branch "3399.mypy"

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?