-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable taskprov in draft07 #436
Merged
Merged
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
cjpatton
requested review from
armfazh,
bhalleycf,
captain-mota,
chris-wood and
mendess
as code owners
November 23, 2023 01:12
cjpatton
force-pushed
the
cjpatton/next-dap-9
branch
2 times, most recently
from
November 23, 2023 01:57
2fd5ccd
to
a8cc3fe
Compare
mendess
requested changes
Nov 23, 2023
mendess
requested changes
Nov 24, 2023
mendess
reviewed
Nov 27, 2023
cjpatton
force-pushed
the
cjpatton/next-dap-9
branch
from
November 28, 2023 19:01
3fd0524
to
f5c1957
Compare
mendess
approved these changes
Nov 29, 2023
cjpatton
force-pushed
the
cjpatton/next-dap-8
branch
from
November 29, 2023 15:29
fb09b07
to
fc59ed2
Compare
cjpatton
force-pushed
the
cjpatton/next-dap-9
branch
2 times, most recently
from
November 29, 2023 16:39
b7ca505
to
2c967c3
Compare
Enable support for taskprov in the latest draft. The main technical hurdle is to ensure the Leader can advertise the taskprov config to the Helper in an HTTP Header. The simplest solution is to add the information to `DapTaskConfig` that is needed in order to reconstruct the serialized taskprov config, in particular the "task_info" field. To do this, we add a new `method` field to the `DapTaskConfig` struct that conveys the method by which the task was configured. The method has type `enum DapTaskConfigMethod`, of which there are two variants. The first is `Taskprov` and includes the "task_info". The second is `Unknown` and is a catch-all for existing, manually configured tasks. Accordingly, we deprecate the `taskprov` field of the `DapTaskConfig` struct. We've kept the field in the struct for backwards compatibility: there are some situations for which the indication is sufficient; in situations where the "task_info" field is needed, we must abort. This change also includes a number of related, but minor changes: * Have `resolve_advertised_task_config()` return a `DapAbort` rather than a `DapError` (an error in this flow always leads to an abort and is never an internal error). * Opt-out of the task unless (1) the "none" variant for "DpConfig" is indicated and (2) the indicated max batch query count is 1. Other values for these parameters are not currently supported by Daphne. * Introduce a type `DapTaskParameters` that encapsulates the parameters used to configure a task and implement a method for generating a task config using the taskprov method. This change is intended to simplify the test code, but should be independently useful.
cjpatton
force-pushed
the
cjpatton/next-dap-9
branch
from
November 29, 2023 16:45
2c967c3
to
b53eda2
Compare
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.
Stacked on #435.
Partially addresses #350.
Enable taskprov in draft07
Enable support for taskprov in the latest draft. The main technical
hurdle is to ensure the Leader can advertise the taskprov config to the
Helper in an HTTP Header. The simplest solution is to add the
information to
DapTaskConfig
that is needed in order to reconstructthe serialized taskprov config, in particular the "task_info" field.
To do this, we add a new
method
field to theDapTaskConfig
structthat conveys the method by which the task was configured. The method has
type
enum DapTaskConfigMethod
, of which there are two variants. Thefirst is
Taskprov
and includes the "task_info". The second isUnknown
and is a catch-all for existing, manually configured tasks.Accordingly, we deprecate the
taskprov
field of theDapTaskConfig
struct. We've kept the field in the struct for backwards compatibility:
there are some situations for which the indication is sufficient; in
situations where the "task_info" field is needed, we must abort.
This change also includes a number of related, but minor changes:
Have
resolve_advertised_task_config()
return aDapAbort
ratherthan a
DapError
(an error in this flow always leads to an abort andis never an internal error).
Opt-out of the task unless (1) the "none" variant for "DpConfig" is
indicated and (2) the indicated max batch query count is 1. Other
values for these parameters are not currently supported by Daphne.
Introduce a type
DapTaskParameters
that encapsulates the parametersused to configure a task and implement a method for generating a task
config using the taskprov method. This change is intended to simplify
the test code, but should be independently useful.