Lint with mypy on CI - #219
Conversation
|
@nihalxkumar You modified some of these files in your PR, I think that it is easier:
Rather than fixing those errors in a separate PR, as that might cause some merge conflicts. |
|
I ran the mypy check on my branch. It reports the same 34 errors already present on main, with no new errors from my changes |
Bodies of untyped functions are not checked, so you can add return type to functions you added to make mypy lint them. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #219 +/- ##
==========================================
- Coverage 71.42% 71.38% -0.05%
==========================================
Files 12 12
Lines 1344 1384 +40
==========================================
+ Hits 960 988 +28
- Misses 384 396 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
🙋 My patch for 213: https://gist.github.com/nihalxkumar/b5d76eb71ba8ebaa45ac90af08d6e9b2 |
I was trying to circumvent a mypy error that entrypoint had two types, |
|
|
||
| def add_new_download( | ||
| self, _user_data, description: str, total_to_download: float | ||
| self, _user_data, description: str, total_to_download: int |
There was a problem hiding this comment.
This (and other changes to function signatures in this class) is wrong - the function is called by libdnf and according to its documentation it uses float here:
| add_new_download(self, user_data, description, total_to_download)
| Notify the client that a new download has been created.
| :type user_data: void
| :param user_data: User data entered together with url/package to download.
| :type description: string
| :param description: The message describing new download (url/packagename).
| :type total_to_download: float
| :param total_to_download: Total number of bytes to download.
| :rtype: void
| :return: Associated user data for new download.
There was a problem hiding this comment.
Thanks. Was this automatic detection somehow that I could have done via editor or some other tool? To fix, I looked at help(libdnf5.repo.DownloadCallbacks.add_new_download).
There was a problem hiding this comment.
Nope, I just checked type changes in this PR that looked suspicious to me.
This looks to be caused by this PR. |
| # silent is equivalent to quiet for dom0-update | ||
| entrypoint.append("--silent") | ||
| entrypoint_cmd.append("--silent") | ||
| entrypoint: list[str] | str = entrypoint_cmd |
There was a problem hiding this comment.
With keeping it a list, secondary variable isn't really needed, please restore to using just entrypoint (just adding type hint should be enough, if mypy can't figure it out on its own).
There was a problem hiding this comment.
update_manager.py:433: error: Item "str" of "list[str] | str" has no attribute "append" [union-attr]
update_manager.py:436: error: Item "str" of "list[str] | str" has no attribute "append" [union-attr]
I added two variables to deal with this. I thought it was better to have a type per variable instead of using # type: ignore.
There was a problem hiding this comment.
What if you let it be just list[str] in this code branch? It should still be compatible with list[str] | str later, no?
There was a problem hiding this comment.
But if that doesn't work, then fine, lets have a separate variable.
There was a problem hiding this comment.
What if you let it be just
list[str]in this code branch? It should still be compatible withlist[str] | strlater, no?
Mypy doesn't like setting type twice for a variable.
update_manager.py:439: error: Name "entrypoint" already defined on line 431 [no-redef]
This is strange. I don't know what used to happen before, because:
It can be empty string though: >>> import os
>>> import tempfile
>>> arch_format = ".tar.gz"
>>> arch_dir = tempfile.mkdtemp()
>>> root_dir = os.path.dirname(None)
Traceback (most recent call last):
File "<python-input-4>", line 1, in <module>
root_dir = os.path.dirname(None)
File "<frozen posixpath>", line 178, in dirname
TypeError: expected str, bytes or os.PathLike object, not NoneType
>>> root_dir = os.path.dirname("")
>>> root_dir
''
>>> base_dir = os.path.basename("".strip(os.sep))
>>> base_dir
''
>>> src_arch = os.path.join(arch_dir, base_dir + arch_format)
>>> src_arch
'/tmp/tmpxxt_4vja/.tar.gz'But this is really weird. |
|
But that's |
|
openQArun TEST=system_tests_network_updates TEST_TEMPLATES=debian-13-xfce UPDATE_TEMPLATES=debian-13-xfce Ok, this repo is not in the allow list... |
|
Regarding last push, you forgot to remove old assert location |
|
Do you know what failed here? QubesOS/qubes-core-admin#783 (comment) |
|
It "worked" (but see the failure there, merge conflict): https://gitlab.com/QubesOS/qubes-continuous-integration/-/pipelines/2740561055 |
Ok, github broke the search syntax
https://github.com/search?q=org%3AQubesOS+is%3Apr+label%3Aopenqa-group-2+is%3Aopen&type=code
The new URL is: And if you notice, the |
5 participants
I'd like mypy to run on #213.