Skip to content
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.

Sign up for GitHub

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

[Storage] Internal avro parser. #10764

Merged
merged 13 commits into from
Apr 23, 2020
Merged

[Storage] Internal avro parser. #10764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
e836c96
initial avro parser
kasobol-msft Apr 9, 2020
32828fa
try fixing test...
kasobol-msft Apr 10, 2020
06f6cba
falling in love with python compatibility...
kasobol-msft Apr 11, 2020
dcec5c8
make linter happy.
kasobol-msft Apr 15, 2020
75433fc
raise StopIteration when there is no more bytes instead of tracking f…
kasobol-msft Apr 15, 2020
a93d501
async avro parser
kasobol-msft Apr 16, 2020
a1524ed
fix syntax for Python 3.5
kasobol-msft Apr 16, 2020
2074fa7
get rid of 'readers_schema' as we only honor schema that has been wri…
kasobol-msft Apr 22, 2020
c6c0a43
pr feedback
kasobol-msft Apr 22, 2020
319f52f
trim unused code.
kasobol-msft Apr 22, 2020
be783ea
pr feedback.
kasobol-msft Apr 22, 2020
3f6b143
simplify skip sync in next.
kasobol-msft Apr 22, 2020
4ce5a8b
move avro tests from _shared.
kasobol-msft Apr 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions sdk/storage/azure-storage-blob/azure/storage/blob/_shared/avro/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# -------------------------------------------------------------------------
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that the contents of the _shared directory must be copied across all storage SDKs (queues/fileshare/datalake/etc). This includes the _shared directory in the test suite. If you feel this should not be in the other SDKs (i.e. that avro will not be a part of queue messages, or datalake etc), then please move this directory up a layer :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@annatisch this code is going to be used in azure-storage-blob and azure-storage-blob-changefeed (that one is work in progress) modules. So shared seems to be better fit. If so do we need to copy avro to all storage modules or only two that take dependency on it?

For the context. It's imperative that we hide avro parser and not expose it. So having it packaged separately and adding as dependency isn't an option unfortunately.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can keep it in shared - but it will be distributed in every storage SDK (queues, datalake, etc).
At some point we will add a CI step to ensure the _shared module is kept consistent between all the SDKs.

If you do not wish this to be included in all the storage SDKs, then you could move it up a level to azure.storage.blob._avro

Copy link
Contributor

@xiafu-msft xiafu-msft Apr 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Anna! @annatisch
We created a package azure-storage-blob-changefeed package which depends on azure-storage-blob. Previously I planned to add _shared folder to azure-storage-blob-changefeed to use avro(that is what I told kamil), but now we plan to directly import _shared.avro from azure-storage-blob so that we don't need to put _shared folder in blob-changefeed pkg.

In the future we will have changefeed feature in fileshare and datalake, so probably we can also import the _shared folder from azure-storage-file-share and azure-storage-file-datalake so we don't need to add _shared folder in azure-storage-fileshare-changefeed and azure-storage-datalake-changefeed(these changefeed packages haven't been created yet)

So I guess probably it makes more sense to put leave the avro in shared folder?
What do you think ^_^?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it stays in _shared.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @xiafu-msft! Yes if the plan is to support this in files and datalake as well, then shared is a good place for it. Could you please duplicate it across the other packages?

Copy link
Contributor

@xiafu-msft xiafu-msft Apr 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Anna! @annatisch
Thanks for your feedback! We still need to tweak the DafaFileReader a bit to fit ChangeFeed feature. After that we will duplicate it! So that will be in another pr.

# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
# --------------------------------------------------------------------------
Loading