From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH v6 1/1] ddns: Add provider Feste-IP.Net Date: Thu, 20 May 2021 12:21:22 +0100 Message-ID: In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1113092301406227577==" List-Id: --===============1113092301406227577== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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 we= ll 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 ask t= hose 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 thi= s provider to test it. > One general question: Should a patch be send as uncommited diff from the st= aging area in before for revision instead of a commited one? No, you should have a branch in your local git repository and commit to 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 a= lready) and the idea of the review process is to submit something that at lea= st works. You can ask all sorts of questions about anything you need to know,= 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 t= ested at all. >=20 > So I would urge you to look at any of the other providers, because what you= 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 that= adds the IPv6 address to a field that is named in that variable. That way, w= e 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 of t= he 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 address= . 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. --===============1113092301406227577==--