[PATCH] samba-tools: add computer subcommand

joeg at catalyst.net.nz joeg at catalyst.net.nz
Thu Mar 15 23:08:09 UTC 2018


Hi Andrew,

Thank you for pointing out the comments issue in the
`join_add_dns_records()` function.

I tried to reuse the code in join.py,  but the context of the usage is
quite different.
It needs some extra effort to modify join.py to support this new case.
Also, that may break existing code.
So I chose to copy that function and modify the new copy.

You are right, the comments are not relevant to this case.
I've trim them out, made some more improvement, and rename the function.

The new patch is attached.

Review appreciated!


On 15/03/18 17:09, Andrew Bartlett wrote:
> On Thu, 2018-03-15 at 11:56 +1300, joeg at catalyst.net.nz wrote:
>> Hi Bjoern,
>>
>> Thank you for making the samba-tools computer patch.
>>
>> According to Andrew and Garming's advice, I extract some of the
>> functions in my previous patch, and created a new patch based on yours:
>>
>> 1. Add `--ip-address` option for the create subcommand, to allow user
>> set DNS A or AAAA records while creating the computer.
>> 2. Delete above DNS records while deleting the computer.
>> 3. Add `--service-principal-name` option for the create command, to
>> allow user set `servicePrincipalName` while creating the computer.
>> 4. Tests.
>>
>> Could you or any one else review this patch? Any feedback are welcome
>> and appreciated!
>>
> Thanks Joe!
>
> I've focussed on your changes in particular, I haven't looked at
> Bjoern's changes yet.
>
> For your changes, can you either share the join_add_dns_records() with
> the code in join.py or trim out the comments to be relevents to the add
> computer case?  
>
> It would be great if the preparatory work to change the owner could be
> moved inside that function also, to keep the logic in the library
> functions and not in the main command.  
>
> Particularly if you can share it with join.py the actual business logic
> should be outside the netcmd stuff, and as a re-usable function.  (See
> how samdb.newcomputer() isn't inline). 
>
> Thanks!
>
> Andrew Bartlett
>

-- 
Joe Guo
joeg at catalyst.net.nz
Catalyst IT

-------------- next part --------------
A non-text attachment was scrubbed...
Name: samba-tools-computer-improve-v2.patch
Type: text/x-patch
Size: 51597 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180316/df8f403b/samba-tools-computer-improve-v2.bin>


More information about the samba-technical mailing list