-
Notifications
You must be signed in to change notification settings - Fork 34
Add Akeyless Secret Store/SSH Plugin #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devel
Are you sure you want to change the base?
Changes from all commits
e10452b
e932273
5124a2d
84f425d
be2e93e
98271f6
0858c70
2ee9014
3179884
ae8022e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,6 +62,15 @@ warn_unused_ignores = true | |
| # crashes with some decorators like `@functools.cache`: | ||
| disallow_any_expr = false | ||
|
|
||
| [mypy-awx_plugins.credentials.akeyless] | ||
| # The akeyless SDK does not ship type stubs. These suppressions are needed | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of them? Why? |
||
| # until the SDK gains proper typing support: | ||
| disallow_any_expr = false | ||
| disallow_any_unimported = false | ||
| disallow_any_explicit = false | ||
| disallow_untyped_calls = false | ||
| warn_return_any = false | ||
|
|
||
| [mypy-awx_plugins.credentials.aws_secretsmanager] | ||
| # crashes with some decorators like `@functools.cache`: | ||
| disallow_any_expr = false | ||
|
|
@@ -106,8 +115,23 @@ disallow_any_expr = false | |
| # crashes with some decorators like `@tox.plugin.impl`: | ||
| disallow_any_expr = false | ||
|
|
||
| [mypy-akeyless] | ||
| # The akeyless SDK does not ship type stubs: | ||
| ignore_missing_imports = true | ||
|
|
||
| [mypy-akeyless.*] | ||
| # The akeyless SDK does not ship type stubs: | ||
| ignore_missing_imports = true | ||
|
|
||
|
Comment on lines
+118
to
+125
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The preferred solution is adding own stubs into the Git tree like https://github.com/ansible/awx_plugins.interfaces/tree/1cec8f60cbb84806ce5cb08c27c301dd86dc866a/_type_stubs. MyPy's already configured to pick them up from the same dir: https://github.com/ansible/awx-plugins/blob/c464cf276b9344056d7f47090bf361c2a8d85b45/.mypy.ini#L36C69-L36C104. |
||
| [mypy-tests.*] | ||
| # crashes with some decorators like `@pytest.mark.parametrize`: | ||
| disallow_any_expr = false | ||
| # fails on `@hypothesis.given()`: | ||
| disallow_any_decorated = false | ||
| # fixture return types like `Callable[..., object]` use `...` as Any: | ||
| disallow_any_explicit = false | ||
|
|
||
| [mypy-tests.akeyless_test] | ||
| # Mock API objects are typed as `object`; test kwargs use generic dict | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather have these ignores inline |
||
| # rather than the specific TypedDicts the plugin functions expect: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't they be casted to whatever type necessary? You've done this in the runtime module already. No need to have multiple approaches for the same thing.. |
||
| disable_error_code = attr-defined, arg-type | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,7 +106,11 @@ py-version = "3.11" | |
| # source root is an absolute path or a path relative to the current working | ||
| # directory used to determine a package namespace for modules located under the | ||
| # source root. | ||
| # source-roots = | ||
| # Setting src/ prevents pylint from adding individual credential module | ||
| # directories to sys.path, which would cause false "module imports itself" | ||
| # warnings and circular-import errors for files with the same name as their | ||
| # third-party dependencies (e.g. akeyless.py vs the akeyless SDK): | ||
| source-roots = ["src"] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is already achieved by the diff --git a/.pylintrc.toml b/.pylintrc.toml
index 3ffdc0e837..39b177a8d8 100644
--- a/.pylintrc.toml
+++ b/.pylintrc.toml
@@ -61,18 +61,19 @@ ignore-patterns = ["^\\.#"]
# This patch injects the project directory into the import path so that the
# local `pytest` plugin can be imported when `pylint-pytest` invokes it when
# exploring the fixtures available:
-init-hook = """
-import os, pathlib, sys
-repo_root_path = pathlib.Path.cwd()
-src_path = repo_root_path / 'src'
-sys.path[:0] = [str(src_path if src_path.exists() else repo_root_path)]
-os.environ['PYTHONPATH'] = sys.path[0]
-"""
+# init-hook = """
+# import os, pathlib, sys
+# repo_root_path = pathlib.Path.cwd()
+# src_path = repo_root_path / 'src'
+# sys.path[:0] = [str(src_path if src_path.exists() else repo_root_path)]
+# os.environ['PYTHONPATH'] = sys.path[0]
+# """
# Use multiple processes to speed up Pylint. Specifying 0 will auto-detect the
# number of processors available to use, and will cap the count on Windows to
# avoid hangs.
-jobs = 0
+# jobs = 0
+jobs = 1
# Control the amount of potential inferred values when inferring a single object.
# This can help the performance when dealing with large functions or complex,
@@ -107,6 +108,9 @@ py-version = "3.11"
# directory used to determine a package namespace for modules located under the
# source root.
# source-roots =
+source-roots = [
+ "src/",
+]
# Allow loading of arbitrary C extensions. Extensions are imported into the
# active Python interpreter and may run arbitrary code.
@@ -435,7 +439,7 @@ disable = [
"disallowed-name",
"duplicate-code",
"fixme",
- "import-outside-toplevel",
+ # "import-outside-toplevel",
"invalid-name",
"line-too-long",
"missing-class-docstring",
@@ -444,11 +448,11 @@ disable = [
"missing-timeout",
"no-else-return",
"no-member",
- "no-name-in-module", # false-positive: https://github.com/pylint-dev/pylint/issues/10147#issuecomment-3946199493
+ # "no-name-in-module", # false-positive: https://github.com/pylint-dev/pylint/issues/10147#issuecomment-3946199493
"no-self-use",
"pointless-string-statement",
"raise-missing-from",
- "relative-beyond-top-level",
+ # "relative-beyond-top-level",
"singleton-comparison",
"too-few-public-methods",
"too-many-branches",But regardless, this seems like something that doesn't belong in this PR but could be submitted separately as a linting config / infra improvement rather than related to a plugin being presented. Would you like to send another PR with just this migration? (the |
||
|
|
||
| # Allow loading of arbitrary C extensions. Extensions are imported into the | ||
| # active Python interpreter and may run arbitrary code. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| Akeyless | ||
| Ansible | ||
| Approle | ||
| async | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,8 @@ | ||
| [pytest] | ||
| # Add src/ to sys.path so tests can import the package even in environments | ||
| # where `pip install -e .` has not been run (e.g. the pre-commit pylint venv): | ||
| pythonpath = src | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is definitely a wrong infra change. Things aren't supposed to be imported from the tree, only from installed envs. Additionally, |
||
|
|
||
| addopts = | ||
| # `pytest-xdist`: | ||
| --numprocesses=auto | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily. There's no reason why a single module can't be turned into a folder with some separation of common helpers and entry points, and maybe types.