Trying again at using the python samba_kcc
abartlet at samba.org
Wed May 20 00:17:46 MDT 2015
On Wed, 2015-05-20 at 16:29 +1200, Douglas Bagnall wrote:
> hi Garming,
> Thanks. I've addressed these things and pushed to
> On 15/05/15 22:27, garming at catalyst.net.nz wrote:
> > Hi,
> > So I've spent some time reviewing some of the patches. The last dozen or
> > so I've left off for now since Douglas hasn't signed off on them yet.
> > I've looked over the changes and they all seem fairly reasonable. I've
> > made my notes in the form of TODO on the tree.
> > Douglas:
> > - The edge list always being empty in your comment is slightly worrying
> Yes, but fortunately I was wrong -- or at least I have since patched
> up and rebased away the thing that made me right.
> > - There's a few commits that should probably be merged without too much
> > trouble
> > - There is a commit that implies RO is partial when it may not be the case?
> That was not about RO at all, but I can see how the comment didn't
> explain that well enough. If the patch had 60 lines of context it
> would be clear. I hope this message improves it:
> (vs the original, with Garming's query:
> https://git.samba.org/?p=garming/samba.git;a=commit;h=8e180eb7ec51 )
> > - There's some code you highlighted that might be dealing with schedules
> > wrong - should it be fixed? And in general, what should None schedules
> > mean? I think you've noted that there are differences with it being always
> > in some places and being that 15 minute block every hour in others - could
> > it always mean the latter?
> I have clarified the comment you highlighted.
> The source of the confusion is that there are two schedule types
> (ReplTimes and Schedule) used by different objects, each packaging the
> same information in different ways. In the specification, both types
> are called "schedule" in variables and attributes, and we followed the
> spec in our naming. In the original C++ these would have been
> disambiguated by the declared types, but in python it is unclear which
> schedule type is in use at any point. A null Schedule tends to mean
> "never" while a null ReplInfo tends to mean "always".
> > The last things I think is that, I see you've removed the SMTP code among
> > other things. But you've also removed some of the failed link and site
> > link bridge code. For the stuff that could be implemented still and isn't
> > stubbed out anymore, it should probably be noted well somewhere. I still
> > think there might be some cases which are executed in the remove
> > unnecessary failed links but we sort of would need a local store before it
> > made more sense.
> Right. I've reverted that one. I think this part:
> - # Remove all tuples in kcc_failed_links where failure count = 0
> - # In this implementation, this should never happen.
> really doesn't apply to us, but I haven't thought through the other bit.
> > I've pushed it with my review to my public Samba git repo:
> > git://git.samba.org/garming/samba.git
> > https://git.samba.org/?p=garming/samba.git;a=shortlog;h=refs/heads/kcc-intersite-to-review
> There is one TODO commit by abartlet at samba.org. Andrew, can you look
> at it?
Yes, I'll fix that up.
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
More information about the samba-technical