Fixing anti-patterns (cherry picks from PR 1039)#1069
Merged
Conversation
Closed
The `-X dev` option reveals useful information such as leaked system resources.
Forgot this.
At minimum this converts the usage to the documented approach where this is exactly one asyncio event loop (started in main) and everything that currently is written async actually becomes a coroutine. The unit tests which use coroutines now use `IsolatedAsyncioTestCase` instead of just `TestCase`, which ensures they continue to work.
Instead of aliasing this function, we should just rename it entirely. Instead of delegating the parsing of an `argparse.Namespace` object, we just setup the `runbook` argument to be a `Path` instead of a string, and have `load_runbook` accept what it needs instead of digging it out of the `Namespace`.
And use single underscore to avoid invoking name mangling. If the name is reused in a subclass, we likely do not intend to have a duplicate variable, but are intending to use it. For variables we expect to be used, we should use Python’s built-in `property` class/decorator. Co-Authored-By: Andy Schwartzmeyer <andrew@schwartzmeyer.com>
Module variables are not recommend, and it brings problems in ut. Refine code to avoid use it.
Co-Authored-By: Andy Schwartzmeyer <andrew@schwartzmeyer.com>
Since it’s really just a list comprehension. Also add a bunch of notes identifying things to investigate further.
There’s more work to do because this was using duck typing. It’s a little better now, but it’s just a massive wrapper for a library that’s just another wrapper for Python’s actual APIs (which are just wrappers for the system’s APIs).
Co-authored-by: Chi Song <27178119+squirrelsc@users.noreply.github.com>
`LisaTestCase` is _not_ an abstract base class, it’s just a base class (with a lot of implementation) and defines no abstract methods. So we remove this.
squirrelsc
force-pushed
the
chisong/v3_pr_1039_picks
branch
from
2c4434d to
b536d96
Compare
Contributor
Author
|
merge due to timeout. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry pick changes from #1039 , and some improvements.