fix getNextAvailableAddress to use local maximums instead of just last a... #168
Conversation
| localMax <- sortedAddresses if !sortedAddresses.contains(localMax + 1) | ||
| ) yield localMax | ||
|
|
||
| localMaximaAddresses.isEmpty match { |
There was a problem hiding this comment.
localMaximaAddresses.headOption.flatMap( localMax => ...)
There was a problem hiding this comment.
Ah awesome. I knew there had to be something like that, but totally forgot about this method. Thanks, that will clean this up
| orderBy(t.address asc) | ||
| ).toSeq | ||
|
|
||
| val localMaximaAddresses = for { |
There was a problem hiding this comment.
make these lazy
scala> lazy val selection = for { i <- Stream.range(0, x.size-2)
| curr = x(i)
| next = x(i+1)
| if (next > curr + 1)
| } yield curr
selection: scala.collection.immutable.Stream[Long] = <lazy>
There was a problem hiding this comment.
Sure thing, makes sense.
|
lgtm 6️⃣ |
|
@maddalab updated, and confirmed that it functions correctly now. Could you please take a look and see if everything makes sense? |
| ).toSeq | ||
|
|
||
| lazy val localMaximaAddresses = for { | ||
| i <- Range(0, sortedAddresses.size-1).inclusive.toStream |
There was a problem hiding this comment.
does that have to be size - 2? either that or remove the inclusive
scala> x
Array[Int] = Array(1, 2, 3)
scala> Range(0, x.size - 1)
scala.collection.immutable.Range = Range(0, 1)
scala> Range(0, x.size - 1).inclusive
scala.collection.immutable.Range = Range(0, 1, 2)
scala> x.size
Int = 3
scala> x(2)
Int = 3
scala> x(2+1)
java.lang.ArrayIndexOutOfBoundsException: 3
|
I haven't tested all the edge cases, but a basic test looks like its working. I can't quite figure out where in the code its going out and finding which addresses are allocated or not - is that what subnetinfo in ipaddresscalc does? |
byxorna
changed the title
|
OK @byxorna answered my question offline. LGTM |
|
@byxorna |
🏁
|
📻 📟 📼 🐻 |
fix getNextAvailableAddress to use local maximums instead of just last a...
...ddress
@dallasmarlow @joshrabinowitz @Primer42 @maddalab Comments/reviews welcome. This intends to fix IP allocation when not using pools. Old behavior would just allocate the next address, regardless of if a lower address was free. It would fail to allocate if you had a range 0..20 and IPs 18,19,20 were in use :(
Ive tested this for edgecases, and believe it is working correctly. However, I would love comments. I have a sneaking suspicion I could do this slickly with a for comprehension, but it escapes me.