From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] ddns: Add provider Feste-IP.Net Date: Fri, 21 May 2021 09:47:41 +0100 Message-ID: In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0079320961645301992==" List-Id: --===============0079320961645301992== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello Martin, > On 20 May 2021, at 17:51, Martin Krieger wrote: >=20 > Hello Michael, >=20 > this is my latest improvement: >=20 > From c745be038550bf66f2c233ff9de1348bcfd99080 Mon Sep 17 00:00:00 2001 > From: Martin Krieger > Date: Tue, 18 May 2021 19:42:44 +0200 > Subject: [PATCH] ddns: Add provider Feste-IP.Net >=20 > Provider supports IPv4, IPv6 & DS (Dual-Stack). > Required url parameter myip for IPv4 or IPv6. > Optional url parameter myip2 for DS which holds IPv4 or IPv6. > Url parameters myip & myip2 will not hold the same record type. > --- > README | 1 + > src/ddns/providers.py | 40 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 41 insertions(+) >=20 > diff --git a/README b/README > index b6decb35c338..fa6ce5e598b8 100644 > --- a/README > +++ b/README > @@ -68,6 +68,7 @@ SUPPORTED PROVIDERS: > easydns.com > enom.com > entrydns.net > + feste-ip.net > freedns.afraid.org > inwx.com|de|at|ch|es > itsdns.de > diff --git a/src/ddns/providers.py b/src/ddns/providers.py > index 56e6620c78ab..8c6da69b6e50 100644 > --- a/src/ddns/providers.py > +++ b/src/ddns/providers.py > @@ -1179,6 +1179,46 @@ class DDNSProviderEntryDNS(DDNSProvider): > raise DDNSUpdateError >=20 >=20 > +class DDNSProviderFesteIPNet(DDNSProtocolDynDNS2, DDNSProvider): > + handle =3D "feste-ip.net" > + name =3D "Feste-IP.Net" > + website =3D "https://www.feste-ip.net/" > + > + # Information about the format of the request is to be found > + # https://forum.feste-ip.net/viewtopic.php?f=3D13&t=3D469 > + > + # myip is required and holds IPv4 or IPv6 > + # myip2 is optional for DS and holds IPv4 or Ipv6 but opposite record typ= e of myip > + myips =3D ("myip","myip2") > + > + url =3D "https://members.feste-ip.net/nic/update/" > + > + def update(self): > + data =3D { > + "hostname" : self.hostname > + } > + > + # idx sets index of myips and checks for required url parameter myip > + idx =3D 0 > + # Loop through DDNSProvider.protocols list > + for proto in DDNSProvider.protocols: > + idx =3D 0 > + tmpip =3D self.get_address(proto) > + # Save only valid address response of protocol proto to the actual url = parameter... > + # and increase idx > + if tmpip: > + data[self.myips[idx]] =3D tmpip > + idx +=3D 1 > + > + # Send DynDNS2 update request if the required url parameter myip is given > + if idx: > + self.send_request(data) > + return > + > + # Raise error if the required if the required url parameter myip gets no= valid address > + raise DDNSUpdateError > + > + > class DDNSProviderFreeDNSAfraidOrg(DDNSProvider): > handle =3D "freedns.afraid.org" > name =3D "freedns.afraid.org" > --=20 > 2.31.0 >=20 > Based on your correct comment idx =3D 0 was on the wrong line, it moved out= side the loop over the protocols (IPv4 & IPv6). You ignored all technical advice that I have given to you. There are solution= s already in the same file that you simply need to copy and paste. They are t= ested, clean and just work. Your implementation is not very Pythonic and raises a random error if no IP a= ddress could be determined. > Pure IPv4 is tested & works as expected. As stated several times I have no = option to test IPv6 and DS on my system. >=20 > The provider serves a test possibility (https://www.feste-ip.net/ddns-servi= ce/faq-ddns/): >=20 > "Kann Ich den Dienst testen ohne mich extra anzumelden? > Ja. Um zu testen ob Ihr Router bzw. Ihr Updateclient mit unserem Dienst fun= ktioniert k=C3=B6nnen Sie folgende Testdaten verwenden: >=20 > DNS-Name: test.feste-ip.net > HOST-ID : 13135 > Passwort: 7sN2KS6L8W >=20 > Bitte beachten Sie dass diese Daten u.U. von mehreren Testern verwendet wer= den. > Ob Ihr Update erfolgreich war k=C3=B6nnen Sie mit der zeitnahen Eingabe von= "nslookup -query=3Dtxt test.feste-ip.net" pr=C3=BCfen." >=20 > Could please anybody in the development-team who has the right technical co= nditions (IPv6 or DS) do test for these cases? That would be nice. Thanks. >=20 > Should I mark my faulty patch on the ipfire git server as superseded? Yes, please. > Regards, >=20 > Martin Krieger >=20 > Michael Tremer: >> Hello, >>> On 20 May 2021, at 12:02, Martin Krieger wrote: >>>=20 >>> Hello Michael, >>>=20 >>> got it. You are right, sorry for confusion. >>>=20 >>> But I really read the process description and have read your comments as = well as looking in providers.py for examples. >>>=20 >>> What should I do? Remove anything, improve it,...? >> Yes, please improve the patch. If there is anything unclear, you should=20 > ask those questions first before sending a fixed version of that patch. >>> Simply issue a request to you to implement that? >> No, this won=E2=80=99t work because we don=E2=80=99t have an account with = this provider to test it. >>> One general question: Should a patch be send as uncommited diff from the = staging area in before for revision instead of a commited one? >> No, you should have a branch in your local git repository and commit to=20 > that. You can use =E2=80=9Cgit commit =E2=80=94-amend=E2=80=9D if you need = to edit an existing commit. >> Best, >> -Michael >>>=20 >>> Regards, >>>=20 >>> Martin Krieger >>>=20 >>>=20 >>>=20 >>> Am 20. Mai 2021 12:00:26 MESZ schrieb Michael Tremer : >>> Hello Martin, >>>=20 >>> Thanks for working on ddns, but *please* read Peter=E2=80=99s email. >>>=20 >>> This is now the third patch (I don=E2=80=99t know why you are at number 6= already) and the idea of the review process is to submit something that at l= east works. You can ask all sorts of questions about anything you need to kno= w, but you will have to listen to what people answer. >>>=20 >>> This patch is fundamentally broken. I will comment on that further below. >>>=20 >>> Secondly, we cannot even think about merging something that isn=E2=80=99t= tested at all. >>>=20 >>> So I would urge you to look at any of the other providers, because what y= ou are trying to implement has already been implemented here for example: >>>=20 >>> https://git.ipfire.org/?p=3Dddns.git;a=3Dblob;f=3Dsrc/ddns/providers.py= ;h=3D56e6620c78ab3d9de1c945147f86e70a8d8614d7;hb=3DHEAD#l975 >>>=20 >>> @Stefan: We might want to have a class variable in the DynDNS protocol=20 > that adds the IPv6 address to a field that is named in that variable. That = way, we avoid any code duplication. >>>=20 >>> On 18 May 2021, at 19:49, Martin Krieger wrote: >>>=20 >>> From 37fb0237932fb12bdd635e9cb5e01b0bf9f03dda Mon Sep 17 00:00:00 2001 >>> From: Martin Krieger >>> Date: Tue, 18 May 2021 19:42:44 +0200 >>> Subject: [PATCH v6 1/1] ddns: Add provider Feste-IP.Net >>>=20 >>> Comments: >>> Provider supports IPv4, IPv6 & DS (Dual-Stack) >>>=20 >>> Changelog: >>>=20 >>> 18.05.2021 >>> Improved failure handling. >>> IPv6 & DS still not checked, because my network connection is pure IPv4. >>>=20 >>> 17.05.2021 >>> 5th attempt ([PATCH v5 1/1]) >>>=20 >>> 16.05.2021 >>> 4th attempt ([PATCH v4 1/1]) >>>=20 >>> 08.05.2021 >>> 3rd attempt ([PATCH v3 1/1]) >>>=20 >>> 06.05.2021 >>> 2nd attempt ([PATCH v2 1/1]) >>>=20 >>> 05.05.2021 >>> 1st attempt ([PATCH v1 1/1]) >>>=20 >>> Signed-off-by: Martin Krieger .. >>> README | 1 + >>> src/ddns/providers.py | 31 +++++++++++++++++++++++++++++++ >>> 2 files changed, 32 insertions(+) >>>=20 >>> diff --git a/README b/README >>> index b6decb35c338..fa6ce5e598b8 100644 >>> --- a/README >>> +++ b/README >>> @@ -68,6 +68,7 @@ SUPPORTED PROVIDERS: >>> easydns.com >>> enom.com >>> entrydns.net >>> + feste-ip.net >>> freedns.afraid.org >>> inwx.com|de|at|ch|es >>> itsdns.de >>> diff --git a/src/ddns/providers.py b/src/ddns/providers.py >>> index 56e6620c78ab..b7e14482a0b1 100644 >>> --- a/src/ddns/providers.py >>> +++ b/src/ddns/providers.py >>> @@ -1179,6 +1179,37 @@ class DDNSProviderEntryDNS(DDNSProvider): >>> raise DDNSUpdateError >>>=20 >>>=20 >>> +class DDNSProviderFesteIPNet(DDNSProtocolDynDNS2, DDNSProvider): >>> + handle =3D "feste-ip.net" >>> + name =3D "Feste-IP.Net" >>> + website =3D "https://www.feste-ip.net/" >>> + >>> + # Information about the format of the request is to be found >>> + # https://forum.feste-ip.net/viewtopic.php?f=3D13&t=3D469 >>> + >>> + myips =3D ("myip","myip2") >>> + >>> + url =3D "https://members.feste-ip.net/nic/update/" >>> + >>> + def update(self): >>> + data =3D { >>> + "hostname" : self.hostname >>> + } >>> + >>> + for proto in DDNSProvider.protocols: >>> + idx =3D 0 >>> + tmpip =3D self.get_address(proto) >>> + if tmpip: >>> + data[self.myips[idx]] =3D tmpip >>> + idx +=3D 1 >>>=20 >>> Setting idx is useless here. You will reset to zero in every iteration=20 > of the loop. Since it is being incremented last and then reset to zero, idx= will *always* be zero. >>>=20 >>> + >>> + if self.myips[0] in data: >>> + self.send_request(data) >>> + return# >>>=20 >>> You will only send one request here and only if it contains an IPv4 addre= ss. Why? What happens to systems that only have IPv6? >>>=20 >>> -Michael >>>=20 >>> + >>> + raise DDNSUpdateError >>> + >>> + >>> class DDNSProviderFreeDNSAfraidOrg(DDNSProvider): >>> handle =3D "freedns.afraid.org" >>> name =3D "freedns.afraid.org" >>> --=20 >>> 2.31.0 >>>=20 >>>=20 >>>=20 >>>=20 >>>=20 >>> --=20 >>> Diese Nachricht wurde von meinem Android-Ger=C3=A4t mit K-9 Mail gesendet. >=20 --===============0079320961645301992==--