Fix bugs related to zero components in sequence#1767
Fix bugs related to zero components in sequence#1767ioana-blue wants to merge 4 commits intoapache:masterfrom
Conversation
|
Fixes #1760 |
|
|
||
| protected[core] case class SequenceExec(components: Vector[FullyQualifiedEntityName]) extends Exec(Exec.SEQUENCE) { | ||
| override def size = components.map(c => c.size).reduce(_ + _) | ||
| override def size = if (components.size > 0) components.map(c => c.size).reduce(_ + _) else (0 B) |
There was a problem hiding this comment.
What about:
components.map(c => c.size).reduceOption(_ + _).getOrElse(0.B)
?
There was a problem hiding this comment.
Or do we even need size calculation on Sequences? Isn't the "size" of the list bound by its count anyway? Could we always return 0 byte?
There was a problem hiding this comment.
didn't know about reduceOption. gracias.
ioana-blue
force-pushed
the
ioana-fix-size
branch
from
7955408 to
7d9e87b
Compare
|
PG3 209 |
|
this is ready to merge as far as I'm concerned @markusthoemmes the ball is in your court |
|
@ioana-blue if this is pg approved, please add corresponding label. |
|
@rabbah can't add label, I'm just a contributor :) |
|
I can't do any assignments on the right hand side of this window... to be more precise |
rabbah
added
pgapproved
|
|
||
| private case class BlockingInvokeTimeout(activationId: ActivationId) extends TimeoutException | ||
| private case class TooManyActionsInSequence() extends RuntimeException | ||
| private case class NoComponentInSequence() extends RuntimeException |
There was a problem hiding this comment.
should these be illegal state exceptions instead of runtime exceptions?
There was a problem hiding this comment.
debatable. IllegalStateException makes me think that I made a request at a bad time (bad state on the server side). these exception look more similar to IllegalArgumentException - I'm performing a request with arguments that don't fit the bill/contract.
I don't feel strongly about this, so I can change them. what's it gonna be? @rr @markusthoemmes
There was a problem hiding this comment.
IllegalArgumentException is what I meant - thanks. That's appropriate. RuntimeExceptions don't fit these.
ioana-blue
force-pushed
the
ioana-fix-size
branch
from
7734cac to
7b3dc85
Compare
|
all done. anything else? |
| val content = WhiskActionPut(Some(Exec.sequence(Vector()))) | ||
|
|
||
| // create an action sequence | ||
| Put(s"$collectionPath/${seqName.name}", content) ~> sealRoute(routes(creds)) ~> check { |
There was a problem hiding this comment.
should also test on update (?overwrite=true).
|
|
||
| protected[core] case class SequenceExec(components: Vector[FullyQualifiedEntityName]) extends Exec(Exec.SEQUENCE) { | ||
| override def size = components.map(c => c.size).reduce(_ + _) | ||
| override def size = components.map(c => c.size).reduceOption(_ + _) getOrElse (0 B) |
There was a problem hiding this comment.
@markusthoemmes once we limit the length of the action names, i'm ok turning this into a no-op (return 0.B).
ioana-blue
force-pushed
the
ioana-fix-size
branch
from
7b3dc85 to
d15461a
Compare
|
PG3 225 approved |
ioana-blue
added
ready
and removed
review
|
Merged as 5ace507. |
3 participants
Don't allow creation of sequences with no components (+ test). Guard size calculation just in case.