Zone file import - support missing params. #126

hhellyer · 2024-06-21T14:22:33Z

This PR adds support for missing parameters when importing zone files:

networks
views
name

As both zone creation with or without a zone file to prepopulate it use the same call, just with or without the zonefile specificed, the "name" field also added to the api to create a zone and "views" is added to the list of fields passed through on zone creation.

Add support for missing parameters when importing zone files: - networks - views - name Support for the "name" field also added to create without zonefile.

hhellyer · 2024-06-25T07:44:58Z

Since this will need an update to the changelog before releasing I've added my proposed changelog update below. (It is a bit wordy because of the need to differentiate between behaviour changes when creating a zone and new things when creating a zone with a zonefile.)

## 0.20.1 (June ??th, 2024)

ENHANCEMENTS:
* Adds support for specifying a list of views when creating zones with or without a provided zone file.
* Adds support for specifying a zone name other than the FQDN when creating zones with or without a provided zone file.
* A specified list of networks for a zone was only applied to zone creation when a zone file was not provided. If a list of networks is specified it will now be applied when creating zones by uploading a zone file as well.

shane-ns1

There's a typo (zone instead of name in a docstring), but more important is that this changes order of parameters passed to the API.

shane-ns1 · 2024-06-25T12:31:02Z

ns1/__init__.py

@@ -371,7 +377,8 @@ def createZone(
        If zoneFile is specified, upload the specific zone definition file
        to populate the zone with.

-        :param str zone: zone name, like 'example.com'
+        :param str zone: zone FQDN, like 'example.com'
+        :param str zone: zone name override, name will be zone FQDN if omitted


Suggested change

:param str zone: zone name override, name will be zone FQDN if omitted

:param str name: zone name override, name will be zone FQDN if omitted

It is possible in Python to declare keyword-only arguments:

https://peps.python.org/pep-3102/

But we don't have that now, so order matters.

If you want to reorder the parameters like this you'll need to make a major release, as it can break the API. Alternately you can put it at the end. 😄

shane-ns1 · 2024-06-25T12:31:44Z

ns1/rest/zones.py

    def _buildBody(self, zone, **kwargs):
        body = {}
        body["zone"] = zone
        self._buildStdBody(body, kwargs)
        return body

    def import_file(
-        self, zone, zoneFile, callback=None, errback=None, **kwargs
+        self, zone, zoneFile, name=None, callback=None, errback=None, **kwargs


Similar to the change in __init__.py, this reorders the parameters and is a breaking change.

shane-ns1 · 2024-06-25T12:34:16Z

ns1/rest/zones.py

+        # parameter but the zonefile API expects parameters containing comma
+        # seperated values.
+        if fields.get("networks") is not None:
+            networksStrs = map(str, fields["networks"])


I guess this is because people may pass networks as numbers?

Our API takes a list of network numbers.
Unfortunately it takes them in a single query parameter of the form networks=1,2,3 which is not how you should pass an array via parameters, as I've found out while working on the other side of our import API, and not how the request library will pass an array. So we need to convert the array to a string.
If allowed to the libs will pass an array using multiple params of the same name:
networks=1&networks=2&networks=3
but our API won't accept that.

shane-ns1 · 2024-06-25T12:34:29Z

ns1/rest/zones.py

+            networksStrs = map(str, fields["networks"])
+            networksParam = ",".join(networksStrs)
+            params["networks"] = networksParam
+        if fields.get("views") is not None:


But we don't expect people to pass views as numbers?

No, views are passed by name.
🤷

shane-ns1 · 2024-06-25T12:35:06Z

ns1/rest/zones.py

+            params["name"] = name
+        return params
+
+    def create(self, zone, name=None, callback=None, errback=None, **kwargs):


Here also, changing the order of parameters may break things for some users.

shane-ns1 · 2024-06-25T12:35:35Z

ns1/zones.py

@@ -90,14 +90,21 @@ def success(result, *args):
            self.zone, callback=success, errback=errback, **kwargs
        )

-    def create(self, zoneFile=None, callback=None, errback=None, **kwargs):
+    def create(
+        self, name=None, zoneFile=None, callback=None, errback=None, **kwargs


This may break the API for folks.

cbertinato · 2024-06-25T13:06:24Z

ns1/rest/zones.py

+        # parameter but the zonefile API expects parameters containing comma
+        # seperated values.
+        if fields.get("networks") is not None:
+            networksStrs = map(str, fields["networks"])


Snake case in Python.

cbertinato · 2024-06-25T13:07:19Z

ns1/rest/zones.py

        return self._make_request(
            "PUT",
-            "%s/%s" % (self.ROOT, zone),
+            "%s/%s" % (self.ROOT, name),


Use f-strings. I know you're just changing a variable name, but might as well update while we're at it.

Done, on this function, create(), and on import(). I've left the others in this file as those were the two I altered and (more importantly) wrote tests for. There aren't specific tests for the other functions in this file.

cbertinato · 2024-06-25T13:07:47Z

tests/unit/test_zone.py

+    if name is not None:
+        params["name"] = name
+
+    zoneFilePath = "{}/../../examples/importzone.db".format(


hhellyer added 4 commits June 21, 2024 15:19

Zone file import - support missing params.

f34ae29

Add support for missing parameters when importing zone files: - networks - views - name Support for the "name" field also added to create without zonefile.

Fix path to sample zone file.

8688bcb

Fix linter errors, improve docs.

753ef66

Fix linter line length errors.

22b2789

shane-ns1 requested changes Jun 25, 2024

View reviewed changes

cbertinato reviewed Jun 25, 2024

View reviewed changes

cbertinato requested changes Jun 25, 2024

View reviewed changes

hhellyer added 2 commits June 28, 2024 16:34

Fix review comments.

5b29159

Use f-strings for formatting.

4728a24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zone file import - support missing params. #126

Zone file import - support missing params. #126

hhellyer commented Jun 21, 2024

hhellyer commented Jun 25, 2024

shane-ns1 left a comment

shane-ns1 Jun 25, 2024

shane-ns1 Jun 25, 2024

shane-ns1 Jun 25, 2024

hhellyer Jun 28, 2024

shane-ns1 Jun 25, 2024

hhellyer Jul 5, 2024

shane-ns1 Jun 25, 2024

shane-ns1 Jun 25, 2024

cbertinato Jun 25, 2024

cbertinato Jun 25, 2024

hhellyer Jul 5, 2024

cbertinato Jun 25, 2024

	:param str zone: zone name override, name will be zone FQDN if omitted
	:param str name: zone name override, name will be zone FQDN if omitted

Zone file import - support missing params. #126

Are you sure you want to change the base?

Zone file import - support missing params. #126

Conversation

hhellyer commented Jun 21, 2024

hhellyer commented Jun 25, 2024

shane-ns1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment