Closed pgalbraith closed 2 years ago
@pgalbraith Apologies for the very slow follow up. This looks good; I added couple of notes but would be happy to merge this in master
for inclusion in 4.1.0 -- there will be a few other additions (for new UUID variants, 6, 7 and 8).
One other thing: if you want, it'd be great to add a note on README.md
for this new added generator method, so others find it more easily.
I think this is fine; I can do minor clean up on my own. :)
Hmmmh. A bunch of unit tests fail for me locally, will revert.
Ok so I had to revert this but would like to try again. But not quite sure how to resurrect merged PR...
I think the problem was that refactoring of fromInterface()
returned null
but caller did not check for null
case.
So depending on order of interfaces returned it could (and did, in my case) result in NPE. Relatively easy to check, I see if I can just manually merge changes.
Ok, so, unfortunately this simply does not work on my machine (macos on Mac mini); egress accessors return null
(or before adding checks, NPE).
I think there is also a small but significant problem with missing null handling, added a note above.
So at this point I cannot really merge this. The idea sounds reasonable so I hope this can be worked to pass tests on different machines with differing interfaces.
Ok thanks for taking a look, hopefully I can take a look as well this weekend and see where it went wrong.
@cowtowncoder ah yes I see the problem, back when I wrote this it was on a different machine, and that machine must have been returning the default interface as the first one in the loop, and things worked fine. My new machine doesn't do that, i have the same problem as you did.
@pgalbraith ok yes that makes sense.
egressTimeBasedGenerator() factory method that will make an educated guess as to which interface to use, by trying to find the interface corresponding to the local default network route