From 5d27d170f288116d942dafd9a7b785e70de89474 Mon Sep 17 00:00:00 2001 From: Justin Okamoto Date: Thu, 8 Oct 2026 18:46:18 +0000 Subject: [PATCH 1/3] Fix EC2 default security group creation when no tags are provided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #471 (https://github.com/dask/dask-cloudprovider/pull/471) added a required `tags` parameter to `create_default_security_group` to support tagging resources at creation time. However, it introduced two bugs: 1. The call site in `get_security_group` was not updated to pass `tags`, causing a `TypeError` at runtime whenever EC2Cluster tried to create a default security group: TypeError: create_default_security_group() missing 1 required positional argument: 'tags' 2. Even after passing `tags=None`, the `TagSpecifications` list was always included in the `CreateSecurityGroup` API call — even when the resulting `Tags` list was empty. AWS rejects this with: InvalidParameterValue: Tag specification must have at least one tag Fix 1: Pass `tags=None` from `get_security_group` to `create_default_security_group`, since `get_security_group` has no tags of its own to forward (callers that do, like ECS, pass tags directly and are unaffected). Fix 2: Only include `TagSpecifications` in the API call when there is at least one non-empty tag to attach. --- dask_cloudprovider/aws/helper.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/dask_cloudprovider/aws/helper.py b/dask_cloudprovider/aws/helper.py index e93ca13a..f824e793 100644 --- a/dask_cloudprovider/aws/helper.py +++ b/dask_cloudprovider/aws/helper.py @@ -82,7 +82,7 @@ async def get_security_group(client, vpc, create_default=True): if create_default: try: return await create_default_security_group( - client, DEFAULT_SECURITY_GROUP_NAME, vpc + client, DEFAULT_SECURITY_GROUP_NAME, vpc, tags=None ) except Exception as e: raise RuntimeError( @@ -95,20 +95,12 @@ async def get_security_group(client, vpc, create_default=True): async def create_default_security_group(client, group_name, vpc, tags): + tag_list = [{"Key": k, "Value": v} for k, v in (tags or {}).items() if k and v] response = await client.create_security_group( Description="A default security group for Dask", GroupName=group_name, VpcId=vpc, - TagSpecifications=[ - { - "ResourceType": "security-group", - "Tags": [ - {"Key": k, "Value": v} - for k, v in (tags or {}).items() - if k and v # Filter out empty tags - ], - } - ], + **({"TagSpecifications": [{"ResourceType": "security-group", "Tags": tag_list}]} if tag_list else {}), DryRun=False, ) From 89859734d00b4c02a82a880ff7382fdb07fe0a6b Mon Sep 17 00:00:00 2001 From: Justin Okamoto Date: Thu, 8 Oct 2026 19:04:28 +0000 Subject: [PATCH 2/3] Ran black formatter on `dask_cloudprovider/aws/helper.py` --- dask_cloudprovider/aws/helper.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dask_cloudprovider/aws/helper.py b/dask_cloudprovider/aws/helper.py index f824e793..4b3ace4a 100644 --- a/dask_cloudprovider/aws/helper.py +++ b/dask_cloudprovider/aws/helper.py @@ -100,7 +100,15 @@ async def create_default_security_group(client, group_name, vpc, tags): Description="A default security group for Dask", GroupName=group_name, VpcId=vpc, - **({"TagSpecifications": [{"ResourceType": "security-group", "Tags": tag_list}]} if tag_list else {}), + **( + { + "TagSpecifications": [ + {"ResourceType": "security-group", "Tags": tag_list} + ] + } + if tag_list + else {} + ), DryRun=False, ) From b7570b971509dc0f6e0ef67e82b11cfee9866af7 Mon Sep 17 00:00:00 2001 From: Justin Okamoto Date: Thu, 8 Oct 2026 19:27:51 +0000 Subject: [PATCH 3/3] Move `external` marks from fixtures to tests for pytest 9 compatibility Test collection fails under pytest 9 with: Failed: Marks cannot be applied to fixtures. See docs: https://docs.pytest.org/en/stable/deprecations.html#applying-a-mark-to-a-fixture-function Several fixtures were decorated with `@pytest.mark.external`. pytest has never applied marks on fixtures to the tests that use them (see https://github.com/pytest-dev/pytest/issues/3664), so these marks were silent no-ops. pytest 7.4 started emitting a deprecation warning for this, and pytest 9.0 turned it into a hard error. The CI environments install pytest unpinned, so they picked up the new behavior. Because the marks were no-ops, the conftest `--create-external-resources` gate never applied to tests that only got the mark through a fixture. Those tests were skipped only at runtime by `skip_without_credentials`. Remove the marks from the fixtures and put `@pytest.mark.external` on the tests that use them, where it was missing. This matches the pattern already used in the Nebius and DigitalOcean tests, and makes the external-resource gate work as intended. --- dask_cloudprovider/aws/tests/test_ec2.py | 10 +++++----- dask_cloudprovider/digitalocean/tests/test_droplet.py | 1 - dask_cloudprovider/hetzner/tests/test_vserver.py | 2 +- dask_cloudprovider/nebius/tests/test_nebius.py | 1 - dask_cloudprovider/openstack/tests/test_instances.py | 2 +- 5 files changed, 7 insertions(+), 9 deletions(-) diff --git a/dask_cloudprovider/aws/tests/test_ec2.py b/dask_cloudprovider/aws/tests/test_ec2.py index 496ed073..896aabc6 100644 --- a/dask_cloudprovider/aws/tests/test_ec2.py +++ b/dask_cloudprovider/aws/tests/test_ec2.py @@ -24,7 +24,6 @@ async def skip_without_credentials(): @pytest.fixture -@pytest.mark.external async def cluster(): await skip_without_credentials() async with EC2Cluster(asynchronous=True) as cluster: @@ -32,7 +31,6 @@ async def cluster(): @pytest.fixture -@pytest.mark.external async def cluster_sync(): await skip_without_credentials() cluster = EC2Cluster() @@ -40,7 +38,6 @@ async def cluster_sync(): @pytest.fixture -@pytest.mark.external async def cluster_rapids(): await skip_without_credentials() async with EC2Cluster( @@ -57,7 +54,6 @@ async def cluster_rapids(): @pytest.fixture -@pytest.mark.external async def cluster_rapids_packer(): await skip_without_credentials() async with EC2Cluster( @@ -74,7 +70,6 @@ async def cluster_rapids_packer(): @pytest.fixture -@pytest.mark.external async def cluster_packer(): await skip_without_credentials() async with EC2Cluster( @@ -99,6 +94,7 @@ async def test_init(): @pytest.mark.asyncio @pytest.mark.timeout(600) +@pytest.mark.external async def test_create_cluster(cluster): assert cluster.status == Status.running @@ -113,6 +109,7 @@ async def test_create_cluster(cluster): @pytest.mark.asyncio @pytest.mark.timeout(600) +@pytest.mark.external async def test_create_cluster_sync(cluster_sync): assert cluster_sync.status == Status.running @@ -125,6 +122,7 @@ async def test_create_cluster_sync(cluster_sync): @pytest.mark.asyncio @pytest.mark.timeout(600) +@pytest.mark.external async def test_create_cluster_with_packer(cluster_packer): assert cluster_packer.status == Status.running @@ -139,6 +137,7 @@ async def test_create_cluster_with_packer(cluster_packer): @pytest.mark.asyncio @pytest.mark.timeout(1200) +@pytest.mark.external async def test_create_rapids_cluster(cluster_rapids): assert cluster_rapids.status == Status.running @@ -158,6 +157,7 @@ def f(): @pytest.mark.asyncio @pytest.mark.timeout(1200) +@pytest.mark.external async def test_create_rapids_cluster_with_packer(cluster_rapids_packer): assert cluster_rapids_packer.status == Status.running diff --git a/dask_cloudprovider/digitalocean/tests/test_droplet.py b/dask_cloudprovider/digitalocean/tests/test_droplet.py index 8be129eb..a67af99f 100644 --- a/dask_cloudprovider/digitalocean/tests/test_droplet.py +++ b/dask_cloudprovider/digitalocean/tests/test_droplet.py @@ -36,7 +36,6 @@ async def config(): @pytest.fixture -@pytest.mark.external async def cluster(config): await skip_without_credentials(config) async with DropletCluster(asynchronous=True) as cluster: diff --git a/dask_cloudprovider/hetzner/tests/test_vserver.py b/dask_cloudprovider/hetzner/tests/test_vserver.py index c1b2af54..a23788bb 100644 --- a/dask_cloudprovider/hetzner/tests/test_vserver.py +++ b/dask_cloudprovider/hetzner/tests/test_vserver.py @@ -36,7 +36,6 @@ async def config(): @pytest.fixture -@pytest.mark.external async def cluster(config): await skip_without_credentials(config) async with HetznerCluster(asynchronous=True) as cluster: @@ -51,6 +50,7 @@ async def test_init(): @pytest.mark.asyncio @pytest.mark.timeout(600) +@pytest.mark.external async def test_create_cluster(cluster): assert cluster.status == Status.running diff --git a/dask_cloudprovider/nebius/tests/test_nebius.py b/dask_cloudprovider/nebius/tests/test_nebius.py index 0d83c21e..fd115bd5 100644 --- a/dask_cloudprovider/nebius/tests/test_nebius.py +++ b/dask_cloudprovider/nebius/tests/test_nebius.py @@ -40,7 +40,6 @@ async def config(): @pytest.fixture -@pytest.mark.external async def cluster(config): await skip_without_credentials(config) async with NebiusCluster(asynchronous=True, debug=True) as cluster: diff --git a/dask_cloudprovider/openstack/tests/test_instances.py b/dask_cloudprovider/openstack/tests/test_instances.py index eae815bf..c71b9f26 100644 --- a/dask_cloudprovider/openstack/tests/test_instances.py +++ b/dask_cloudprovider/openstack/tests/test_instances.py @@ -34,7 +34,6 @@ async def config(): @pytest.fixture -@pytest.mark.external async def cluster(config): await skip_without_credentials(config) @@ -50,6 +49,7 @@ async def test_init(): @pytest.mark.asyncio @pytest.mark.timeout(600) +@pytest.mark.external async def test_create_cluster(cluster): assert cluster.status == Status.running cluster.scale(1)