From 9b0169be3590f32a25ca2f79427373f0e36f028f Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Fri, 12 Jun 2020 19:41:56 +0000 Subject: [PATCH] Fix pylint(no-member) when accessing resource.id (#4813) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pylint currently reports `E1101: Instance of 'Bucket' has no 'id' member (no-member)` on lines in Pulumi Python programs like: ```python pulumi.export('bucket_name', bucket.id) ``` Here's a description of this message from http://pylint-messages.wikidot.com/messages:e1101: > Used when an object (variable, function, …) is accessed for a non-existent member. > > False positives: This message may report object members that are created dynamically, but exist at the time they are accessed. This appears to be a false positive case: `id` isn't set in the constructor (it's set later in `register_resource`) and Pylint isn't able to figure this out statically. `urn` has the same problem. (Oddly, Pylint doesn't complain when accessing other resource output properties). This change refactors `register_resource` so that `id` and `urn` can be assigned in the resource's constructor, so that Pylint can see it being assigned. The change also does the same with `read_resource`. --- CHANGELOG.md | 3 + sdk/python/lib/pulumi/resource.py | 15 ++++- sdk/python/lib/pulumi/runtime/resource.py | 64 ++++++++++++++----- tests/integration/integration_test.go | 33 ++++++++++ tests/integration/python/pylint/.gitignore | 5 ++ tests/integration/python/pylint/Pulumi.yaml | 3 + tests/integration/python/pylint/__main__.py | 32 ++++++++++ .../python/pylint/requirements.txt | 1 + 8 files changed, 138 insertions(+), 18 deletions(-) create mode 100644 tests/integration/python/pylint/.gitignore create mode 100644 tests/integration/python/pylint/Pulumi.yaml create mode 100644 tests/integration/python/pylint/__main__.py create mode 100644 tests/integration/python/pylint/requirements.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 358525527..da890c744 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ CHANGELOG [#4812](https://github.com/pulumi/pulumi/pull/4812) +- Fix `pylint(no-member)` when accessing `resource.id`. + [#4813](https://github.com/pulumi/pulumi/pull/4813) + ## 2.4.0 (2020-06-10) - Turn program generation NYIs into diagnostic errors [#4794](https://github.com/pulumi/pulumi/pull/4794) diff --git a/sdk/python/lib/pulumi/resource.py b/sdk/python/lib/pulumi/resource.py index 3386d2ec4..83a001fb9 100644 --- a/sdk/python/lib/pulumi/resource.py +++ b/sdk/python/lib/pulumi/resource.py @@ -18,7 +18,7 @@ from typing import Optional, List, Any, Mapping, Union, Callable, TYPE_CHECKING, import copy from .runtime import known_types -from .runtime.resource import register_resource, register_resource_outputs, read_resource +from .runtime.resource import _register_resource, register_resource_outputs, _read_resource from .runtime.settings import get_root_resource from .metadata import get_project, get_stack @@ -706,9 +706,18 @@ class Resource: if not custom: raise Exception( "Cannot read an existing resource unless it has a custom provider") - read_resource(cast('CustomResource', self), t, name, props, opts) + res = cast('CustomResource', self) + result = _read_resource(res, t, name, props, opts) + res.urn = result.urn + assert result.id is not None + res.id = result.id else: - register_resource(self, t, name, custom, props, opts) + result = _register_resource(self, t, name, custom, props, opts) + self.urn = result.urn + if custom: + assert result.id is not None + res = cast('CustomResource', self) + res.id = result.id def _convert_providers(self, provider: Optional['ProviderResource'], providers: Optional[Union[Mapping[str, 'ProviderResource'], List['ProviderResource']]]) -> Mapping[str, 'ProviderResource']: if provider is not None: diff --git a/sdk/python/lib/pulumi/runtime/resource.py b/sdk/python/lib/pulumi/runtime/resource.py index c3a20e153..34eca8d1e 100644 --- a/sdk/python/lib/pulumi/runtime/resource.py +++ b/sdk/python/lib/pulumi/runtime/resource.py @@ -139,10 +139,21 @@ async def prepare_resource(res: 'Resource', aliases, ) + +class _ResourceResult(NamedTuple): + urn: Output[str] + """ + The URN of the resource. + """ + id: Optional[Output[str]] = None + """ + The id of the resource, if it's a CustomResource, otherwise None. + """ + + # pylint: disable=too-many-locals,too-many-statements - -def read_resource(res: 'CustomResource', ty: str, name: str, props: 'Inputs', opts: 'ResourceOptions'): +def _read_resource(res: 'CustomResource', ty: str, name: str, props: 'Inputs', opts: 'ResourceOptions') -> _ResourceResult: if opts.id is None: raise Exception( "Cannot read resource whose options are lacking an ID value") @@ -162,7 +173,7 @@ def read_resource(res: 'CustomResource', ty: str, name: str, props: 'Inputs', op urn_secret.set_result(False) resolve_urn = urn_future.set_result resolve_urn_exn = urn_future.set_exception - res.urn = known_types.new_output({res}, urn_future, urn_known, urn_secret) + result_urn = known_types.new_output({res}, urn_future, urn_known, urn_secret) # Furthermore, since resources being Read must always be custom resources (enforced in the # Resource constructor), we'll need to set up the ID field which will be populated at the end of @@ -174,7 +185,7 @@ def read_resource(res: 'CustomResource', ty: str, name: str, props: 'Inputs', op resolve_value: asyncio.Future[Any] = asyncio.Future() resolve_perform_apply: asyncio.Future[bool] = asyncio.Future() resolve_secret: asyncio.Future[bool] = asyncio.Future() - res.id = known_types.new_output( + result_id = known_types.new_output( {res}, resolve_value, resolve_perform_apply, resolve_secret) def do_resolve(value: Any, perform_apply: bool, exn: Optional[Exception]): @@ -263,16 +274,23 @@ def read_resource(res: 'CustomResource', ty: str, name: str, props: 'Inputs', op asyncio.ensure_future(RPC_MANAGER.do_rpc("read resource", do_read)()) + return _ResourceResult(result_urn, result_id) + +def read_resource(res: 'CustomResource', ty: str, name: str, props: 'Inputs', opts: 'ResourceOptions') -> None: + result = _read_resource(res, ty, name, props, opts) + res.urn = result.urn + assert result.id is not None + res.id = result.id + + # pylint: disable=too-many-locals,too-many-statements - -def register_resource(res: 'Resource', ty: str, name: str, custom: bool, props: 'Inputs', opts: Optional['ResourceOptions']): - """ - registerResource registers a new resource object with a given type t and name. It returns the - auto-generated URN and the ID that will resolve after the deployment has completed. All - properties will be initialized to property objects that the registration operation will resolve - at the right time (or remain unresolved for deployments). - """ +def _register_resource(res: 'Resource', + ty: str, + name: str, + custom: bool, + props: 'Inputs', + opts: Optional['ResourceOptions']) -> _ResourceResult: log.debug(f"registering resource: ty={ty}, name={name}, custom={custom}") monitor = settings.get_monitor() @@ -289,17 +307,17 @@ def register_resource(res: 'Resource', ty: str, name: str, custom: bool, props: urn_secret.set_result(False) resolve_urn = urn_future.set_result resolve_urn_exn = urn_future.set_exception - res.urn = known_types.new_output({res}, urn_future, urn_known, urn_secret) + result_urn = known_types.new_output({res}, urn_future, urn_known, urn_secret) # If a custom resource, make room for the ID property. + result_id = None resolve_id: Optional[Callable[[ Any, bool, Optional[Exception]], None]] = None if custom: - res = cast('CustomResource', res) resolve_value: asyncio.Future[Any] = asyncio.Future() resolve_perform_apply: asyncio.Future[bool] = asyncio.Future() resolve_secret: asyncio.Future[bool] = asyncio.Future() - res.id = known_types.new_output( + result_id = known_types.new_output( {res}, resolve_value, resolve_perform_apply, resolve_secret) def do_resolve(value: Any, perform_apply: bool, exn: Optional[Exception]): @@ -433,6 +451,22 @@ def register_resource(res: 'Resource', ty: str, name: str, custom: bool, props: asyncio.ensure_future(RPC_MANAGER.do_rpc( "register resource", do_register)()) + return _ResourceResult(result_urn, result_id) + +def register_resource(res: 'Resource', ty: str, name: str, custom: bool, props: 'Inputs', opts: Optional['ResourceOptions']) -> None: + """ + Registers a new resource object with a given type t and name. It returns the + auto-generated URN and the ID that will resolve after the deployment has completed. All + properties will be initialized to property objects that the registration operation will resolve + at the right time (or remain unresolved for deployments). + """ + result = _register_resource(res, ty, name, custom, props, opts) + res.urn = result.urn + if custom: + assert result.id is not None + res = cast('CustomResource', res) + res.id = result.id + def register_resource_outputs(res: 'Resource', outputs: 'Union[Inputs, Output[Inputs]]'): async def do_register_resource_outputs(): diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 006974dd9..1722b1318 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -1738,3 +1738,36 @@ func TestLargeResourceDotNet(t *testing.T) { Dir: filepath.Join("large_resource", "dotnet"), }) } + +// Test to ensure Pylint is clean. +func TestPythonPylint(t *testing.T) { + var opts *integration.ProgramTestOptions + opts = &integration.ProgramTestOptions{ + Dir: filepath.Join("python", "pylint"), + Dependencies: []string{ + filepath.Join("..", "..", "sdk", "python", "env", "src"), + }, + ExtraRuntimeValidation: func(t *testing.T, stack integration.RuntimeValidationStackInfo) { + randomURN := stack.Outputs["random_urn"].(string) + assert.NotEmpty(t, randomURN) + + randomID := stack.Outputs["random_id"].(string) + randomVal := stack.Outputs["random_val"].(string) + assert.Equal(t, randomID, randomVal) + + cwd := stack.Outputs["cwd"].(string) + assert.NotEmpty(t, cwd) + + pylint := filepath.Join("venv", "bin", "pylint") + if runtime.GOOS == WindowsOS { + pylint = filepath.Join("venv", "Scripts", "pylint") + } + + err := integration.RunCommand(t, "pylint", []string{pylint, "__main__.py"}, cwd, opts) + assert.NoError(t, err) + }, + Quick: true, + UseAutomaticVirtualEnv: true, + } + integration.ProgramTest(t, opts) +} diff --git a/tests/integration/python/pylint/.gitignore b/tests/integration/python/pylint/.gitignore new file mode 100644 index 000000000..3f47d8e79 --- /dev/null +++ b/tests/integration/python/pylint/.gitignore @@ -0,0 +1,5 @@ +*.pyc +/.pulumi/ +/dist/ +/*.egg-info +venv/ diff --git a/tests/integration/python/pylint/Pulumi.yaml b/tests/integration/python/pylint/Pulumi.yaml new file mode 100644 index 000000000..04777cb9c --- /dev/null +++ b/tests/integration/python/pylint/Pulumi.yaml @@ -0,0 +1,3 @@ +name: pylint +description: A simple Python program that should be pylint clean. +runtime: python diff --git a/tests/integration/python/pylint/__main__.py b/tests/integration/python/pylint/__main__.py new file mode 100644 index 000000000..c93252ed7 --- /dev/null +++ b/tests/integration/python/pylint/__main__.py @@ -0,0 +1,32 @@ +# Copyright 2016-2020, Pulumi Corporation. All rights reserved. + +"""An example program that should be Pylint clean""" + +import binascii +import os +import pulumi +from pulumi.dynamic import Resource, ResourceProvider, CreateResult + + +class RandomResourceProvider(ResourceProvider): + """Random resource provider.""" + + def create(self, props): + val = binascii.b2a_hex(os.urandom(15)).decode("ascii") + return CreateResult(val, {"val": val}) + + +class Random(Resource): + """Random resource.""" + val: str + + def __init__(self, name, opts=None): + super().__init__(RandomResourceProvider(), name, {"val": ""}, opts) + + +r = Random("foo") + +pulumi.export("cwd", os.getcwd()) +pulumi.export("random_urn", r.urn) +pulumi.export("random_id", r.id) +pulumi.export("random_val", r.val) diff --git a/tests/integration/python/pylint/requirements.txt b/tests/integration/python/pylint/requirements.txt new file mode 100644 index 000000000..7fb0ea150 --- /dev/null +++ b/tests/integration/python/pylint/requirements.txt @@ -0,0 +1 @@ +pylint