Usability of 'samba-tool domain provision'
Simo
simo at samba.org
Wed Jun 29 14:12:29 UTC 2016
>From the code pov it's all ok, but indentation is poor (not that the
original code is in good shape).
Can we follow PEP8[1] which is the official python style and is
followed by most of the good python projects ?
There are tools that auctomatically check for python code style like
pylint, any chance we can start using them and enforcing a good style?
Simo.
[1]https://www.python.org/dev/peps/pep-0008/
On Wed, 2016-06-29 at 12:33 +0100, Rowland Penny wrote:
> On 29/06/16 09:20, Rowland Penny wrote:
> >
> > On 29/06/16 02:47, Simo wrote:
> > >
> > > Mostly nitpicks and syntax, otherweise looks good, see comments
> > > inline
> > > please.
> > OK, see my replies inline
> >
> > >
> > > Simo.
> > >
> > > On Tue, 2016-06-28 at 16:11 -0700, Jeremy Allison wrote:
> > > >
> > > > On Mon, Jun 06, 2016 at 04:26:52PM +0100, Rowland Penny wrote:
> > > > >
> > > > > On 06/06/16 08:04, Rowland Penny wrote:
> > > > > >
> > > > > > On 06/06/16 06:41, Andrew Bartlett wrote:
> > > > > > >
> > > > > > > On Fri, 2016-06-03 at 13:44 +0100, Rowland Penny wrote:
> > > > > > > >
> > > > > > > > From d5ce8d2545731f825a18b094eb86992b24dddd75 Mon Sep
> > > > > > > > 17
> > > > > > > > 00:00:00
> > > > > > > > 2001
> > > > > > > > From: Rowland Penny <rpenny at samba.org>
> > > > > > > > Date: Thu, 2 Jun 2016 15:41:51 +0100
> > > > > > > > Subject: [PATCH] samba-tool domain provision, remove
> > > > > > > > unused
> > > > > > > > server roles.
> > > > > > > As per --use-xattrs, can we start by marking this
> > > > > > > deprecated?
> > > > > > >
> > > > > > > That will avoid needing to restructure the tests right
> > > > > > > now
> > > > > > > (which do
> > > > > > > use this option), and avoid breaking scripts that might
> > > > > > > specify
> > > > > > > the -
> > > > > > > -server-role=dc default.
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Andrew Bartlett
> > > > > > >
> > > > > > OK, I will also add something to the main help, along the
> > > > > > lines
> > > > > > of
> > > > > > 'only provisioning a DC actually works'.
> > > > > >
> > > > > > Rowland
> > > > > >
> > > > > OK, lets try again, please see attached patches.
> > > > Ping. Can someone on the Team with python expertise review
> > > > Rowland's
> > > > patches
> > > > please ?
> > > >
> > > > I'd do it, but I'd really have to learn more python first :-).
> > > >
> > > > Jeremy.
> > > >
> > > > >
> > > > > From 5ad2103038f8109906764d108af8422c1fef694e Mon Sep 17
> > > > > 00:00:00
> > > > > 2001
> > > > > From: Rowland Penny <rpenny at samba.org>
> > > > > Date: Mon, 6 Jun 2016 15:43:59 +0100
> > > > > Subject: [PATCH 1/5] samba-tool domain provision, mark '
> > > > > --server-
> > > > > role' option
> > > > > as depreciated and don't ask for server_role if run
> > > > > interactively.
> > > > >
> > > > > Signed-off-by: Rowland Penny <rpenny at samba.org>
> > > > > ---
> > > > > python/samba/netcmd/domain.py | 4 ++--
> > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/python/samba/netcmd/domain.py
> > > > > b/python/samba/netcmd/domain.py
> > > > > index fd26d93..703441b 100644
> > > > > --- a/python/samba/netcmd/domain.py
> > > > > +++ b/python/samba/netcmd/domain.py
> > > > > @@ -221,7 +221,7 @@ class cmd_domain_provision(Command):
> > > > > choices=["fedora-ds", "openldap"]),
> > > > > Option("--server-role", type="choice",
> > > > > metavar="ROLE",
> > > > > choices=["domain controller", "dc", "member
> > > > > server", "member", "standalone"],
> > > > > - help="The server role (domain controller |
> > > > > dc |
> > > > > member server | member | standalone). Default is dc.",
> > > > > + help="The server role (domain controller |
> > > > > dc |
> > > > > member server | member | standalone). Default is dc.
> > > > > (depreciated)",
> > > depreciated -> deprecated
> > I must learn how to spell deprecated :-)
> >
> > >
> > >
> > > >
> > > > >
> > > > > default="domain controller"),
> > > > > Option("--function-level", type="choice",
> > > > > metavar="FOR-
> > > > > FUN-LEVEL",
> > > > > choices=["2000", "2003", "2008",
> > > > > "2008_R2"],
> > > > > @@ -348,7 +348,7 @@ class cmd_domain_provision(Command):
> > > > > if domain is None:
> > > > > raise CommandError("No domain set!")
> > > > > - server_role = ask("Server Role (dc, member,
> > > > > standalone)", "dc")
> > > > > + server_role = "dc"
> > > Why stop asking ?
> > >
> > > In the comment I'd like the rationale.
> > Because, from my tests and what I was told, you can only provision
> > a
> > DC, anything else will not actually work.
> > I will add to the comment.
> >
> > >
> > >
> > > >
> > > > >
> > > > > dns_backend = ask("DNS backend (SAMBA_INTERNAL,
> > > > > BIND9_FLATFILE, BIND9_DLZ, NONE)", "SAMBA_INTERNAL")
> > > > > if dns_backend in (None, ''):
> > > > > --
> > > > > 1.7.10.4
> > > > >
> > > > >
> > > > > From cb917fa550770cfa2687b909f4d52f68743b4054 Mon Sep 17
> > > > > 00:00:00
> > > > > 2001
> > > > > From: Rowland Penny <rpenny at samba.org>
> > > > > Date: Mon, 6 Jun 2016 16:01:11 +0100
> > > > > Subject: [PATCH 2/5] samba-tool domain provision, make
> > > > > 'domain'
> > > > > help a bit
> > > > > more friendly, it also checks if the domain name is not
> > > > > the same as the 'realm' name.
> > > > >
> > > > > Signed-off-by: Rowland Penny <rpenny at samba.org>
> > > > > ---
> > > > > python/samba/netcmd/domain.py | 13 ++++++++++---
> > > > > 1 file changed, 10 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/python/samba/netcmd/domain.py
> > > > > b/python/samba/netcmd/domain.py
> > > > > index 703441b..0ca4c7f 100644
> > > > > --- a/python/samba/netcmd/domain.py
> > > > > +++ b/python/samba/netcmd/domain.py
> > > > > @@ -173,7 +173,7 @@ class cmd_domain_provision(Command):
> > > > > takes_options = [
> > > > > Option("--interactive", help="Ask for names",
> > > > > action="store_true"),
> > > > > Option("--domain", type="string",
> > > > > metavar="DOMAIN",
> > > > > - help="NetBIOS domain name to use"),
> > > > > + help="The NetBIOS domain name to use (also
> > > > > known
> > > > > as 'workgroup'"),
> > > > > Option("--domain-guid", type="string",
> > > > > metavar="GUID",
> > > > > help="set domainguid (otherwise random)"),
> > > > > Option("--domain-sid", type="string",
> > > > > metavar="SID",
> > > > > @@ -348,6 +348,10 @@ class cmd_domain_provision(Command):
> > > > > if domain is None:
> > > > > raise CommandError("No domain set!")
> > > > > + if realm == domain:
> > > > > + raise CommandError("The NetBIOS domain name
> > > > > cannot
> > > > > be the same \
> > > > > +as the realm")
> > > > > +
> > > > > server_role = "dc"
> > > > > dns_backend = ask("DNS backend
> > > > > (SAMBA_INTERNAL,
> > > > > BIND9_FLATFILE, BIND9_DLZ, NONE)", "SAMBA_INTERNAL")
> > > > > @@ -375,9 +379,12 @@ class cmd_domain_provision(Command):
> > > > > else:
> > > > > realm = sambaopts._lp.get('realm')
> > > > > if realm is None:
> > > > > - raise CommandError("No realm set!")
> > > > > + raise CommandError("You must supply the
> > > > > realm
> > > > > name!")
> > > > > if domain is None:
> > > > > - raise CommandError("No domain set!")
> > > > > + raise CommandError("You must supply the
> > > > > NetBIOS
> > > > > domain name!")
> > > > > + if realm == domain:
> > > > > + raise CommandError("The NetBIOS domain name
> > > > > cannot
> > > > > be the same \
> > > > > +as the realm")
> > > Windows defaults the netbios domain name to the domain/realm,
> > > chopping
> > > away anything after the first dot IIRC, maybe we should do the
> > > same ?
> > Does windows allow the setting of a netbios domain name ? What I
> > mean
> > is it always set to the first part of the domain/realm. If it
> > isn't
> > settable, then we should follow windows and I will alter the code.
> >
> > >
> > >
> > > >
> > > > >
> > > > > if not adminpass:
> > > > > self.logger.info("Administrator password will
> > > > > be set
> > > > > randomly!")
> > > > > --
> > > > > 1.7.10.4
> > > > >
> > > > >
> > > > > From 1504d00b28effaf74b981b7755dbb116cad0a277 Mon Sep 17
> > > > > 00:00:00
> > > > > 2001
> > > > > From: Rowland Penny <rpenny at samba.org>
> > > > > Date: Mon, 6 Jun 2016 16:05:29 +0100
> > > > > Subject: [PATCH 3/5] samba-tool domain provision, check
> > > > > length of
> > > > > NetBIOS
> > > > > domain name. This fixes bug 11925
> > > > >
> > > > > Signed-off-by: Rowland Penny <rpenny at samba.org>
> > > > > ---
> > > > > python/samba/netcmd/domain.py | 7 +++++++
> > > > > 1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/python/samba/netcmd/domain.py
> > > > > b/python/samba/netcmd/domain.py
> > > > > index 0ca4c7f..130a3b9 100644
> > > > > --- a/python/samba/netcmd/domain.py
> > > > > +++ b/python/samba/netcmd/domain.py
> > > > > @@ -348,6 +348,10 @@ class cmd_domain_provision(Command):
> > > > > if domain is None:
> > > > > raise CommandError("No domain set!")
> > > > > + if len(domain) > 15:
> > > > > + raise CommandError("The maximum length of
> > > > > the
> > > > > NetBIOS domain \
> > > > > +name is 15 characters")
> > > > > +
> > > > > if realm == domain:
> > > > > raise CommandError("The NetBIOS domain name
> > > > > cannot
> > > > > be the same \
> > > > > as the realm")
> > > > > @@ -382,6 +386,9 @@ as the realm")
> > > > > raise CommandError("You must supply the
> > > > > realm
> > > > > name!")
> > > > > if domain is None:
> > > > > raise CommandError("You must supply the
> > > > > NetBIOS
> > > > > domain name!")
> > > > > + if len(domain) > 15:
> > > > > + raise CommandError("The maximum length of
> > > > > the
> > > > > NetBIOS domain \
> > > > > +name is 15 characters")
> > > > > if realm == domain:
> > > > > raise CommandError("The NetBIOS domain name
> > > > > cannot
> > > > > be the same \
> > > > > as the realm")
> > > > > --
> > > > > 1.7.10.4
> > > > >
> > > > >
> > > > > From ecb9bba0bc37768fc6f085853d89ec65c82f5cb1 Mon Sep 17
> > > > > 00:00:00
> > > > > 2001
> > > > > From: Rowland Penny <rpenny at samba.org>
> > > > > Date: Mon, 6 Jun 2016 16:15:58 +0100
> > > > > Subject: [PATCH 4/5] samba-tool domain provision, make
> > > > > 'interactive' ask if
> > > > > 'use_rfc2307' should be used. It also sets 'use_xattrs'
> > > > > to 'auto' if not set on the command line.
> > > > >
> > > > > Signed-off-by: Rowland Penny <rpenny at samba.org>
> > > > > ---
> > > > > python/samba/netcmd/domain.py | 10 +++++++++-
> > > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/python/samba/netcmd/domain.py
> > > > > b/python/samba/netcmd/domain.py
> > > > > index 130a3b9..e885562 100644
> > > > > --- a/python/samba/netcmd/domain.py
> > > > > +++ b/python/samba/netcmd/domain.py
> > > > > @@ -235,7 +235,7 @@ class cmd_domain_provision(Command):
> > > > > help="Set target directory"),
> > > > > Option("--ol-mmr-urls", type="string",
> > > > > metavar="LDAPSERVER",
> > > > > help="List of LDAP-URLS [
> > > > > ldap://<FQHN>:<PORT>/ (where <PORT> has to be different than
> > > > > 389!)
> > > > > ] separated with comma (\",\") for use with OpenLDAP-MMR
> > > > > (Multi-
> > > > > Master-Replication), e.g.:
> > > > > \"ldap://s4dc1:9000,ldap://s4dc2:9000\""),
> > > > > - Option("--use-xattrs", type="choice",
> > > > > choices=["yes",
> > > > > "no", "auto"], help="Define if we should use the native fs
> > > > > capabilities or a tdb file for storing attributes likes
> > > > > ntacl, auto
> > > > > tries to make an inteligent guess based on the user rights
> > > > > and
> > > > > system capabilities", default="auto"),
> > > > > + Option("--use-xattrs", type="choice",
> > > > > choices=["yes",
> > > > > "no", "auto"], help="Define if we should use the native fs
> > > > > capabilities or a tdb file for storing attributes likes
> > > > > ntacl, auto
> > > > > tries to make an inteligent guess based on the user rights
> > > > > and
> > > > > system capabilities (depreciated)", default="auto"),
> > > depreciated -> deprecated
> > > also can you please split this string on 80 chars limit so it is
> > > legible in common editors and does not cause autowrapping ?
> > Will do
> >
> > >
> > >
> > > >
> > > > >
> > > > > Option("--use-rfc2307", action="store_true",
> > > > > help="Use AD
> > > > > to store posix attributes (default = no)"),
> > > > > ]
> > > > > @@ -321,6 +321,9 @@ class cmd_domain_provision(Command):
> > > > > if len(self.raw_argv) == 1:
> > > > > interactive = True
> > > > > + if use_xattrs is None:
> > > > > + use_xattrs = "auto"
> > > > > +
> > > > > if interactive:
> > > > > from getpass import getpass
> > > > > import socket
> > > > > @@ -368,6 +371,11 @@ as the realm")
> > > > > suggested_forwarder = None
> > > > > dns_forwarder = None
> > > > > + if use_rfc2307 is None:
> > > > > + use_rfc2307 = ask("Use AD to store posix
> > > > > attributes (yes, no)", "no")
> > > > > + if use_rfc2307 in (None, ''):
> > > > > + raise CommandError("Use rfc2307 not
> > > > > set!")
> > > > > +
> > > > > while True:
> > > > > adminpassplain = getpass("Administrator
> > > > > password:
> > > > > ")
> > > > > if not adminpassplain:
More information about the samba-technical
mailing list