[RFC PATCH v5] add JSON output to net ads
Andrew Bartlett
abartlet at samba.org
Fri Aug 24 00:37:41 UTC 2018
On Wed, 2018-08-22 at 13:12 +0200, Philipp Gesang wrote:
> -<| Quoting Andrew Bartlett <abartlet at samba.org>, on Wednesday, 2018-08-22 09:12:30 AM |>-
> > On Tue, 2018-08-21 at 08:29 +0200, Philipp Gesang via samba-technical
> > wrote:
> > > Hey guys,
> > >
> > > this iteration addresses the comments to v4 by Andrew Bartlett
> > > (tests, merge conflicts).
> > >
> > > CI: https://gitlab.com/phgsng/samba/pipelines/28267867
> > >
> > > A note on the tests: Instead of making the code that parses the
> > > non-JSON output more permissive I chose to make the output of
> > > “net ads lookup” more regular. As a consequence, The blackbox
> > > tests now depend on this change. I’m aware that this change is
> > > not entirely related to the patch series. If it is not acceptable
> > > let me know and I’ll tweak the test code accordingly.
> >
> > G'Day Philipp,
> >
> > I'm still stumped as to how your patches differ from the ones that Gary
> > had where we just couldn't make --json work.
>
> No idea, sorry.
>
> > Can you push it to the https://gitlab.com/samba/devel/samba repo so we
> > can see the full test run please?
>
> You mean https://gitlab.com/samba-team/devel/samba ?
> ^^^^^^^^^^
> Done: https://gitlab.com/samba-team/devel/samba/pipelines/28429561
Thanks.
> > Can you please ensure your tests catch exceptions. Allowing
> > BlackboxProcessError seems quite wrong, that is exactly the type of
> > error you should be looking for.
>
> Ok, will do. I saw other tests wrapped to catch
> BlackboxProcessError and assumed it signified a state where
> something was wrong with the harness or environment.
Sort of. Yes, catching BlackboxProcessError is a good thing to do, but
you must raise an proper selftest failure in that case.
> > Finally if you run this against chgdcpass rather than the main
> > ad_dc_ntvfs and ad_dc environments you will have it run in your more
> > limited CI environment. In a comment just explain that as this does
> > not log in, we can spread the load and run it against another
> > environment.
> > (Only the build_samba_ad_dc_2 runs in private repos, sadly the rest of
> > the AD DC tests don't fit in the smaller free VMs provided by
> > gitlab.com).
>
> Ok, will do.
Finally, please don't use 'j', just have a long option --json. Scripts
parsing JSON don't mind having to type extra characters and this avoids
issues with ambigious short options.
Thanks!
Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
https://catalyst.net.nz/services/samba
More information about the samba-technical
mailing list