[wait-for] lambda support #12196
Merged
[wait-for] lambda support #12196
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The following is a very initial idea for adding lambda support to for each queries. It has not been generalised yet, but essentially it should be doable. The idea is that we have a series of scopes that a scope can add to when we enter a lambda. This is kind of like stack registers, but not as complicated. The reason we don't need to implement stack registers is because we're using the go runtime to do it for us, so we can piggy back on to that without having to do any of the work! Additional work to change Ord to Box. The Box value is more generalised and not only talks about Ord syntax, but syntax we need to do other things like accessor values. The idea of Box is the same as Java, except there is no bottom type.
Previously I was aiming to do `applications.forEach`, but actually that
introduces a lot of complexity in to the query evaluator, so much so it
makes it really hard to understand. Instead nothing can have a method
and we use high order functions to do this for us. We don't get partial
application, but I don't think we need it tbh.
The code is rather simple, lift the sub entity into it's own scope and
then use that when we call the lambda. We get a lot of things for free
and it works suprisingly well.
The accessor is identical to the infix in terms of using the order
precedence, but we have a new AST type so that the query evaluator
doesn't become a mess.
juju-wait-for model default --query='forEach(applications, app =>
app.status == "active")'
As there is no order in the forEach, it might be wise to order them to
get some valid expected behaviour.
The generic scope is no longer required.
It is now possible to do...
juju-wait-for model default --query='status == "available" &&
forEach(applications, app => app.status == "active")'
Previously we ended up with app.status being unset
The foreach abstraction is more generalised so we don't pollute the box type. This makes it much easier to implement new box types without having to have empty foreach methods. Additionally added machines to the model.
The following adds more test cases to the query tests
The item intends to order some of the for each calls to make it a lot more predictable.
SimonRichardson
force-pushed
the
wait-for-lambda-support
branch
from
9b20e40
to
0f8a760
Compare
manadart
reviewed
Nov 2, 2020
The order of AllWatcher deltas might not be in an expected order, to which the derived status of the application might not have been processed if the unit delta hasn't arrived. To fix this cache the model and force the revaluation every time, which is worse for performance, but because it's on the client side, it shouldn't matter too much. Yet still this is another time that the application status change has forced client side changes.
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The following is a very initial idea for adding lambda support for
each query.
The idea is that we have a series of scopes that a scope can add to when
we enter a lambda. This is kind of like stack registers, but not as
complicated. The reason we don't need to implement stack registers is
that we're using the go runtime to do it for us, so we can piggyback
on to that without having to do any of the work!
Additional work to change Ord to Box. The Box value is more generalized
and not only talks about Ord syntax but syntax we need to do other
things like accessor values.
The idea of Box is the same as Java, except there is no bottom type.
Previously I was aiming to do
applications.forEach, but actually thatintroduces a lot of complexity into the query evaluator, so much so it
makes it really hard to understand. Instead, nothing can have a method
and we use high order functions to do this for us. We don't get the partial
application, but I don't think we need it tbh.
The code is rather simple, lift the sub-entity into its own scope and
then use that when we call the lambda. We get a lot of things for free
and it works surprisingly well.
The accessor is identical to the infix in terms of using the order
precedence, but we have a new AST type so that the query evaluator
doesn't become a mess.
As there is no order in the forEach, it might be wise to order them to
get some valid expected behavior.
QA steps
juju-wait-for model default --query='forEach(applications, app => app.status == "active")'