Implement Batch Query Commitements RFC #816
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
sergiimk
commented
Sep 7, 2024
| @@ -24,54 +25,59 @@ use url::Url; | |||
| #[derive(Debug, Clone, Merge, Serialize, Deserialize)] | |||
| #[serde(deny_unknown_fields, rename_all = "camelCase")] | |||
| pub struct CLIConfig { | |||
| /// Database connection configuration | |||
| pub database: Option<DatabaseConfig>, | |||
There was a problem hiding this comment.
Sorry for the diff - sorted fields alphabetically
sergiimk
commented
Sep 7, 2024
| @@ -276,6 +285,208 @@ async fn test_data_query_handler_full() { | |||
|
|
|||
| //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// | |||
|
|
|||
| #[test_group::group(engine, datafusion)] | |||
| #[test_log::test(tokio::test)] | |||
| async fn test_data_query_handler_v2() { | |||
There was a problem hiding this comment.
This test explains it all
sergiimk
commented
Sep 8, 2024
Comment on lines
+164
to
+171
| let mut visitors: [&mut dyn MetadataChainVisitor<Error = vis::Infallible>; 6] = [ | ||
| &mut attachments_visitor, | ||
| &mut info_visitor, | ||
| &mut license_visitor, | ||
| &mut schema_visitor, | ||
| &mut seed_visitor, | ||
| &mut vocab_visitor, | ||
| ]; |
There was a problem hiding this comment.
FYI @s373r I'm super happy about this trick:
- Notice that all response elements are optional - so if user did not request
seedorschemawe ideally don't want to search for this event - So I made all visitors above optional
- I then implemented
MetadataChainVisitorforOption<T> where T: MetadataChainVisitor - If the
OptionisNone- the visitor will immediatelyStop - If
Some- it delegates the calls to inner visitor
This is about as concise as it gets :)
There was a problem hiding this comment.
@sergiimk , 💯
Awesome idea with the optional visitors -- looking at the changes in the other PR noticed this, very cool!
sergiimk
force-pushed
the
feature/batch-query-commitments
branch
from
c2ebb5e
to
c324c3e
Compare
sergiimk
force-pushed
the
feature/batch-query-commitments
branch
from
50a28aa
to
427e73e
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.
Description
Closes: kamu-data/kamu-node#125
Closes: kamu-data/kamu-node#126
Implements response signing and a new commitment scheme according to the RFC.
Note that the old response format is preserved for compatibility via
V1 / V2enum trick. If user request in the new format (e.g. by including?include=proof) only then they will get a response in the new format. This will allow us to transition to the new format progressively, without breaking the oracles for example.Also introduces a new
{dataset}/metadataendpoint that allows getting dataset schema, license, info etc.Checklist before requesting a review
Extra steps taken to preserve the compatibility
New config options added, but they have defaults
Will be done separately
DI update is needed and new configuration needs to be exposed - will be preparing a PR