Scaleway ip info by philipnorton42 · Pull Request #11629 · ansible-collections/community.general

philipnorton42 · 2026-03-20T16:24:59Z

SUMMARY

Following on from discussions in the ticket #11351 I have refactored the main scaleway.py class and the scaleway_ip_info module to use "zone" ID, rather than zone names over the old "region" argument. In addition, it's best to move to a system where the URL is constructed, rather than hard coded.

I have added some logic (through annotations) to allow either zone or region to be used, region still functions in the same way as it always has, but will now issue a deprecation warning.

Since the IP info module is a information only module I've selected this as the starting point for refactoring the scaleway modules in this project. They all use different styles of code and make use of the aforemenionted hard coded URLs list.

Whilst I was there, I've also added the new zone that Scaleway have to the main scaleway.py class.

ISSUE TYPE

Refactoring Pull Request

COMPONENT NAME

scaleway_ip_info

ADDITIONAL INFORMATION

New usage of the scaleway_ip_info module is as follows:

    - name: Get ip information and register it in a variable
      community.general.scaleway_ip_info:
        zone: it-mil-1
        api_token: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
      register: ips

…ilding the url, instead of looking up the zone and url from a list. This allows for more flexibility and potential versioning of the end point.

…he region.

ansibullbot · 2026-03-20T16:44:30Z

cc @Spredzy @abarbare @remyleone
click here for bot help

…les that use this setting.

ansibullbot · 2026-03-20T17:05:50Z

cc @Lunik @guillaume_ro_fr
click here for bot help

felixfontein

Thanks for your contribution!

changelogs/fragments/11351-scaleway-ip-info.yml

plugins/modules/scaleway_ip_info.py

russoz

Hi @philipnorton42

Thanks for your contribution! Couple of comments.

plugins/modules/scaleway_container_info.py

russoz · 2026-03-23T18:52:25Z

plugins/modules/scaleway_ip_info.py

-      - Scaleway region to use (for example C(par1)).
-    required: true
+      - Scaleway region to use (for example V(par1)).
+    required: false


required=false is redundant both here and in the arg specs

Yup, good call. Sorted them as well.

russoz · 2026-03-23T18:58:05Z

Oh and please add a changelog fragment.

felixfontein · 2026-03-23T19:03:00Z

Also please note that the extra tests are failing.

… redundant.

…_info module.

philipnorton42 · 2026-03-24T09:58:01Z

Thanks for the feedback, I really appreciate it.
I've popped this PR back into draft whilst I get those changes in, and fix those tests.

russoz

Oops, sorry I didn't see this in time.

In any case, LGTM

…y file.

philipnorton42 · 2026-03-30T16:33:59Z

Apologies for the delay in updating this.

I have added a change record for a minor_changes record for the addition of the zone parameter to the scaleway_ip_info module. After some problems (to do with my local environment) I was able to also get the nox tests to run, and committed the changes it made.

I think that's everything covered here. Please let me know if there is something I've missed :)

felixfontein · 2026-03-30T16:34:26Z

changelogs/fragments/11351-scaleway-ip-info.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - scaleway_ip_info - now accepts a ``zone`` parameter, detailing the Scaleway zone to use in the lookup. This is mutually exclusive of the existing ``region`` parameter (https://github.com/ansible-collections/community.general/issues/11351).


Suggested change

- scaleway_ip_info - now accepts a ``zone`` parameter, detailing the Scaleway zone to use in the lookup. This is mutually exclusive of the existing ``region`` parameter (https://github.com/ansible-collections/community.general/issues/11351).

- scaleway_ip_info - now accepts a ``zone`` parameter, detailing the Scaleway zone to use in the lookup. This is mutually exclusive of the existing ``region`` parameter (https://github.com/ansible-collections/community.general/issues/11351, https://github.com/ansible-collections/community.general/pull/11629).

And there should be entries for the changes in all the modules below.

felixfontein · 2026-03-30T16:43:05Z

plugins/module_utils/scaleway.py


    def send(self, method, path, data=None, headers=None, params=None):
        url = self._url_builder(path=path, params=params)
-        self.warn(url)


I guess this is a bugfix? If yes, it should be mentioned in bugfixes: in the changelog fragment.

Personally I don't think adding/removing log messages should count as a bugfix.

Now instead of removing this entirely, it might be worth replacing that with:

Suggested change

self.warn(url)

self.debug(url)

Showing the URL queried as a warning (which is likely very annoying to users of the modules) looks like forgotten debug code, and removing that would count (and should be announced to the users) as a bugfix IMO.

I thought it looked like left over debugging code that wasn't removed. The fact that it just randomly prints the url being used inside the ansible logs without any context seemed to support this.

Changing this to self.module.debug() seems to be the preferred option from this module (and the rest of the codebase). I'll change it to that.

I did try to change it to use the verbosity flag of the ansible task (using display.vv() etc), but for some reason the library wasn't working on my environment. From what I read it's something to do with the underlying ansible-core library so debug() is probably the way to go (at least for now).

russoz

Hi @philipnorton42

Thanks for your continued efforts here. 12.5.0 has left the building last week.

The new region in Italy is definitely something that could/should be backported to -12 and to -11, IMHO. That would be a bugfix.

The other changes would likely fall in the minor_changes category.

Not sure it's worth the effort of splitting this in two though.

russoz · 2026-03-30T18:27:57Z

plugins/modules/scaleway_ip_info.py

+    type: str
+    description:
+      - Scaleway zone to use (for example V(nl-ams-1)).
+    version_added: 12.5.0


That ship has sailed

Suggested change

version_added: 12.5.0

version_added: 12.6.0

felixfontein · 2026-03-30T18:39:56Z

The new region in Italy is definitely something that could/should be backported to -12 and to -11, IMHO. That would be a bugfix.

In the past, we alwasy treated new regions as a new feature.

russoz

another couple of comments

russoz · 2026-03-31T03:30:17Z

plugins/module_utils/scaleway.py


    def send(self, method, path, data=None, headers=None, params=None):
        url = self._url_builder(path=path, params=params)
-        self.warn(url)


Personally I don't think adding/removing log messages should count as a bugfix.

Now instead of removing this entirely, it might be worth replacing that with:

Suggested change

self.warn(url)

self.debug(url)

russoz · 2026-03-31T03:32:17Z

changelogs/fragments/11351-scaleway-ip-info.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - scaleway_ip_info - now accepts a ``zone`` parameter, detailing the Scaleway zone to use in the lookup. This is mutually exclusive of the existing ``region`` parameter (https://github.com/ansible-collections/community.general/issues/11351).


And there should be entries for the changes in all the modules below.

…thod of the Scaleway class.

…_ip_info module to 12.6.0.

philipnorton42 · 2026-03-31T09:50:20Z

So should I remove all of the updates for the new region and create a new issue for that?

Sorry, I saw the new region being added and thought I would include it, but I can always remove it and create a separate issue for it if that is best?

If not, I'll get the change records updated for this PR.

russoz · 2026-03-31T21:16:12Z

I'd sayit hinges on the verdict for that warn(): if it sticks to a minor change, the new region is OK here, but you'll need to add entries for each module in the changelog fragment.

If it becomes a bugfix, then the region add should go to a different PR.

@felixfontein?

felixfontein · 2026-04-01T04:52:43Z

Why not simply move the warn change to another PR, instead of everything else?

russoz · 2026-04-01T06:42:15Z

well, that would make life much easier :-)

This comment was marked as outdated.

Sign in to view

ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! module module needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html new_contributor Help guide this first time contributor labels Mar 20, 2026

Philip Norton added 4 commits March 20, 2026 16:34

Updated the scaleway_ip_info to use the scaleway zone directly and bu…

91e3e11

…ilding the url, instead of looking up the zone and url from a list. This allows for more flexibility and potential versioning of the end point.

Added the new it-mil-1 and it-mil region to the scaleway utility file.

b61c02c

Updated the scaleway_ip_info test to reflect the usage of zone over t…

a526f84

…he region.

Added changelog fragment for the scaleway_ip_info change.

47ce914

philipnorton42 force-pushed the scaleway_ip_info branch from 72db510 to 47ce914 Compare March 20, 2026 16:34

This comment was marked as outdated.

Sign in to view

Philip Norton added 2 commits March 20, 2026 16:58

Fixing coding standards issues in the scaleway_ip_info.py module.

0ff768d

Added the it-mil region to module documentation for all scaleway modu…

f49dc75

…les that use this setting.

This comment was marked as outdated.

Sign in to view

ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Mar 20, 2026

philipnorton42 marked this pull request as draft March 20, 2026 17:11

ansibullbot added the WIP Work in progress label Mar 20, 2026

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-12 Automatically create a backport for the stable-12 branch labels Mar 21, 2026

felixfontein reviewed Mar 22, 2026

View reviewed changes

changelogs/fragments/11351-scaleway-ip-info.yml Outdated Show resolved Hide resolved

plugins/modules/scaleway_ip_info.py Outdated Show resolved Hide resolved

plugins/modules/scaleway_ip_info.py Outdated Show resolved Hide resolved

philipnorton42 marked this pull request as ready for review March 23, 2026 12:17

ansibullbot removed WIP Work in progress needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Mar 23, 2026

russoz reviewed Mar 23, 2026

View reviewed changes

Removed a required: false for the scaleway_ip_info module since it is…

f6eb4fb

… redundant.

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Mar 24, 2026

Removed calls to required=false from the arg specs in the scaleway_ip…

b87f01e

…_info module.

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Mar 24, 2026

philipnorton42 marked this pull request as draft March 24, 2026 09:55

ansibullbot added the WIP Work in progress label Mar 24, 2026

russoz approved these changes Mar 24, 2026

View reviewed changes

Philip Norton added 2 commits March 30, 2026 15:34

Formatting corrections in the scaleway.py file and scaleway_ip_info.p…

8704b0f

…y file.

Added a change record for the scaleway_ip_info zone parameter addition.

94bfcab

philipnorton42 marked this pull request as ready for review March 30, 2026 16:34

ansibullbot removed the WIP Work in progress label Mar 30, 2026

felixfontein reviewed Mar 30, 2026

View reviewed changes

russoz reviewed Mar 30, 2026

View reviewed changes

russoz reviewed Mar 31, 2026

View reviewed changes

Philip Norton added 2 commits March 31, 2026 10:45

Added a debug() call for the Scaleway URL being used in the send() me…

cf7adef

…thod of the Scaleway class.

Updated the version of the zone parameter being added to the scaleway…

9a36dfd

…_ip_info module to 12.6.0.

ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Apr 9, 2026

		@@ -0,0 +1,2 @@
		minor_changes:
		- scaleway_ip_info - now accepts a ``zone`` parameter, detailing the Scaleway zone to use in the lookup. This is mutually exclusive of the existing ``region`` parameter (https://github.com/ansible-collections/community.general/issues/11351).

Conversation

philipnorton42 commented Mar 20, 2026

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

ansibullbot commented Mar 20, 2026

Uh oh!

ansibullbot commented Mar 20, 2026

Uh oh!

This comment was marked as outdated.

felixfontein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

russoz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

russoz commented Mar 23, 2026

Uh oh!

felixfontein commented Mar 23, 2026

Uh oh!

philipnorton42 commented Mar 24, 2026

Uh oh!

russoz left a comment

Choose a reason for hiding this comment

Uh oh!

philipnorton42 commented Mar 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

russoz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixfontein commented Mar 30, 2026

Uh oh!

russoz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philipnorton42 commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

russoz commented Mar 31, 2026

Uh oh!

felixfontein commented Apr 1, 2026

Uh oh!

russoz commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

philipnorton42 commented Mar 31, 2026 •

edited

Loading