Add queued ingest client with support for ingesting from blobs #29
Conversation
Author: Krishan Mistry <krishan@krishanmistry.com>
…ly identity mapping
* Azure SDK for Rust dependencies to 0.16 * changes
There was a problem hiding this comment.
Great Contribution!
Overall the code is very clean and well commented.
There are a few things to polish - error handilng, lack of tests (though the ones that exist are great), crate managements.
And of course missing options and ingestion methods, which may be out of the scope of this PR.
How would you like to proceed?
Do you expect to work on that more and polish it, or do you want us to take it from here?
| use azure_kusto_data::models::TableV1; | ||
|
|
||
| /// Helper to get a column index from a table | ||
| // TODO: this could be moved upstream into Kusto Data - would likely result in a change to the API of this function to return an Option<usize> |
There was a problem hiding this comment.
Probably a good idea, we need a better interface for tables in general
There was a problem hiding this comment.
I've changed the syntax so it would be ready to move into the data crate - I think the move itself can be done in a separate PR
azure-kusto-ingest/src/resource_manager/authorization_context.rs
Outdated
Show resolved
Hide resolved
azure-kusto-ingest/src/resource_manager/ingest_client_resources.rs
Outdated
Show resolved
Hide resolved
|
Thanks for taking the time to review @AsafMah On how we proceed: I'd ideally like to see this through to completion. I'm sure I'll learn something!
On the missing options and methods, those are out of scope for this PR. I do believe that it is worth considering at this point how they fit into the framework this PR provides. |
|
@krishanjmistry for now you can go over the comments I left, that would be most of it. |
* Use thiserror * use scopes over explicit use of drop * update syntax * remove std::fmt::Debug * qualify thiserror
* Ingest: Azure SDK changes for 0.19 * remove default features on data dependency in ingest * Format Rust code using rustfmt --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
Hi @AsafMah For starters, I think we'd want to migrate to workspace dependencies |
azure-kusto-ingest/src/resource_manager/ingest_client_resources.rs
Outdated
Show resolved
Hide resolved
* resource_uri changes * markups in client_options * resource_manager markups * remove dependency on chrono - use time * cache changes * add missing dev dependency feature * add some basic tests for caching implementation
|
@AsafMah, I believe I've addressed all of your most recent comments |
|
I apologize for my lack of attention. It seems that currently the branch is in conflict with main - can you rebase or merge? |
|
@AsafMah
|
|
That's a general issues with secrets in github with external pull requests, sadly. |
What?
Adds a Kusto queued ingest client which supports ingesting from a blob
Things I'm unsure of:
Crate design
Should ingestion functionality be a separate crate or a feature under a combined "data and ingest" crate?
Error handling
Currently this crate uses
anyhow- this isn't ideal for a library. I noticed there's a customErrorin the data crate but haven't put much thought into how it could be used. I think it would also differ depending on the crate designTesting