Skip to content

Changefeed #10755

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

Jump to bottom
Merged
xiafu-msft merged 13 commits into from
Jun 24, 2020
Merged

Changefeed #10755

xiafu-msft merged 13 commits into from
Jun 24, 2020

Conversation

@xiafu-msft
Copy link
Contributor

@xiafu-msft xiafu-msft commented Apr 9, 2020

No description provided.

@xiafu-msft xiafu-msft changed the base branch from master to feature/storage-stg73 April 17, 2020 17:55
@seanmcc-msft
Copy link
Member

seanmcc-msft commented Apr 17, 2020

Is this the entire Change Feed implementation?

@xiafu-msft
Copy link
Contributor Author

xiafu-msft commented Apr 17, 2020

Is this the entire Change Feed implementation?

It's not! Only a draft pr!

@xiafu-msft xiafu-msft force-pushed the changefeed branch 2 times, most recently from 5b9faa8 to f74956f Compare April 17, 2020 19:01
annatisch
annatisch reviewed Apr 20, 2020
View reviewed changes
sdk/storage/azure-storage-blob-changefeed/azure/storage/blobchangefeed/_models.py Outdated
Copy link
Member

@annatisch annatisch Apr 20, 2020

Choose a reason for hiding this comment

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

This could be moved out of the class into a utility function?

@xiafu-msft xiafu-msft force-pushed the changefeed branch 3 times, most recently from d181575 to 0fa4243 Compare May 2, 2020 00:46
@xiafu-msft xiafu-msft force-pushed the feature/storage-stg73 branch from c8b24fe to 8f45e1a Compare May 2, 2020 00:47
@xiafu-msft xiafu-msft force-pushed the feature/storage-stg73 branch from 8f45e1a to d08d639 Compare May 12, 2020 21:16
annatisch
annatisch requested changes May 14, 2020
View reviewed changes
@annatisch annatisch added the Storage Storage Service (Queues, Blobs, Files) label May 22, 2020
xiafu-msft
xiafu-msft commented May 27, 2020
View reviewed changes
@xiafu-msft xiafu-msft added the APIChange This PR contains an addition or change to the API signature and must be reviewed by an architect. label Jun 3, 2020
@xiafu-msft xiafu-msft force-pushed the feature/storage-stg73 branch from a4f697b to 60ada0b Compare June 10, 2020 22:11
xiafu-msft
xiafu-msft commented Jun 10, 2020
View reviewed changes
annatisch
annatisch reviewed Jun 18, 2020
View reviewed changes
sdk/storage/azure-storage-blob-changefeed/MANIFEST.in
@@ -0,0 +1,6 @@
include *.md
include azure/__init__.py
include azure/storage/__init__.py
Copy link
Member

@annatisch annatisch Jun 18, 2020

Choose a reason for hiding this comment

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

You also need to add:

include azure/storage/blob/__init__.py

Copy link
Contributor Author

@xiafu-msft xiafu-msft Jun 18, 2020

Choose a reason for hiding this comment

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

good catch

annatisch
annatisch reviewed Jun 18, 2020
View reviewed changes
sdk/storage/azure-storage-blob-changefeed/mypy.ini Outdated

# Per-module options:

[mypy-azure.storage.blob._generated.*]
Copy link
Member

@annatisch annatisch Jun 18, 2020

Choose a reason for hiding this comment

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

Is this necessary?

Copy link
Contributor

@rakshith91 rakshith91 Jun 18, 2020

Choose a reason for hiding this comment

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

i believe yes when using the latest autorest.

Copy link
Contributor Author

@xiafu-msft xiafu-msft Jun 18, 2020

Choose a reason for hiding this comment

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

@annatisch I think you are right, we don't need this since there's no _generated folder in changefeed package.
while why do we need [mypy-azure.core.*] will mypy throw error in dependency package?

annatisch
annatisch reviewed Jun 18, 2020
View reviewed changes
sdk/storage/ci.yml
ServiceDirectory: storage
Artifacts:
- name: azure_storage_blob
safeName: azurestorageblob
Copy link
Member

@annatisch annatisch Jun 18, 2020

Choose a reason for hiding this comment

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

Why was this removed?

annatisch
annatisch reviewed Jun 18, 2020
View reviewed changes
annatisch
annatisch reviewed Jun 18, 2020
View reviewed changes
annatisch
annatisch reviewed Jun 18, 2020
View reviewed changes
annatisch
annatisch reviewed Jun 18, 2020
View reviewed changes
sdk/storage/azure-storage-blob-changefeed/README.md Outdated
#### Clients

The Blob ChangeFeed SDK provides one client:
ChangeFeedClient: get change feed events by page, get all change feed events, get events in a time range, start listing events with a continuation token
Copy link
Member

@annatisch annatisch Jun 18, 2020

Choose a reason for hiding this comment

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

This section isn't rendering quite right - I think it needs a bullet point:
https://github.com/Azure/azure-sdk-for-python/blob/bae3f4aefd644fb974e016ab3fb355b19ad15316/sdk/storage/azure-storage-blob-changefeed/README.md#clients

Also description needs a capital and a final period.

annatisch
annatisch reviewed Jun 18, 2020
View reviewed changes
rakshith91
rakshith91 reviewed Jun 18, 2020
View reviewed changes
sdk/storage/azure-storage-blob-changefeed/azure/storage/blob/changefeed/_models.py
self.unprocessed_segment_paths = []
return None
return Segment(self.client, segment_path, page_size, segment_cursor)
return None
Copy link
Contributor

@rakshith91 rakshith91 Jun 18, 2020

Choose a reason for hiding this comment

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

why do we need the last return None?

Copy link
Contributor Author

@xiafu-msft xiafu-msft Jun 19, 2020

Choose a reason for hiding this comment

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

self.current_segment = self._get_next_segment(.....)
because if there's no Segment we need to mark self.current_segment = None

@rakshith91
Copy link
Contributor

rakshith91 commented Jun 22, 2020

you should be able to resolve the lint error once you rebase

@xiafu-msft xiafu-msft merged commit c5f1413 into Azure:feature/storage-stg73 Jun 24, 2020
xiafu-msft added a commit that referenced this pull request Jul 7, 2020
@xiafu-msft @kasobol-msft
@zezha-msft @annatisch
Feature/storage stg73 (#12272)
900aa7d 
* [Storage]STG73

* [Blob][Swagger]Update Swagger (#10943)

* [Blob][Swagger]Regenerate Swagger Code

* fix container test failure caused by list_containers include type change

* [Storage] Internal avro parser. (#10764)

* initial avro parser

* try fixing test...

* falling in love with python compatibility...

* make linter happy.

* raise StopIteration when there is no more bytes instead of tracking file length.

* async avro parser

* fix syntax for Python 3.5

* get rid of 'readers_schema' as we only honor schema that has been written to file ('writer_schema').

* pr feedback

* trim unused code.

* pr feedback.

* simplify skip sync in next.

* move avro tests from _shared.

* Jumbo blob support (#11176)

* wip

* initial test coverage.

* wip.

* wip

* single upload.

* add async tests.

* disable 50k block tests.

* datalake append.

* async datalake

* disable tests that send large payload over network.

* pr feedback.

* Undelete share (#11394)

* Undelete container (#11339)

* [Storage][Blob] Added support for Object Replication (#11525)

* Blob versioning (#11154)

* [Blob][QuickQuery]Add Quick Query Support (#10946)

* [Blob][STG73]Blob Tags (#11418)

* regenerate code (#11964)

* fix the bug which caused only showing fatal error (#11997)

* [Storage][STG73]Address API Review Comments (#12111)

* [Storage][STG73]Address API Review Comments

* [Storage][STG73]dict<policy, rules> -> list(ObjectReplicationPolicy)

* fix blob tag_value test

* expose ObjectReplicationPolicy and ObjectReplicationRule, fix test

* fix test

* Changefeed (#10755)

* [ChangeFeed]Add ChangeFeed Package

* test_avro failure

* update dev_requirement.txt

* change namespace to azure.storage.blob.changefeed

* address comments

* optimize memory when reading changefeed events

* namespace change

* set up package change

* fix failed tests

* readme and kwargs

* Update sdk/storage/azure-storage-blob-changefeed/azure/storage/blob/changefeed/_change_feed_client.py

Co-authored-by: Rakshith Bhyravabhotla <[email protected]>

* address comments

* 'azure-storage-blob>=12.3.0' which does not match the frozen requirement 'azure-storage-blob~=1.3'

Co-authored-by: Rakshith Bhyravabhotla <[email protected]>

* [Storage-Blob] Quick Query API (#11991)

* Renamed query error

* Renamed query reader

* Updated config models

* Updated format request params

* Updated iterator

* fix the bug which caused only showing fatal error

* Updated Error message

* Fixed query helper

* Started test conversion

* small fix

* Fixed tests

* Updated error handling + json model

* Updated recordings

* Removed old recording

* Added iter tests

* Iter test recordings

* Fix test

* Remove extra recording

* Fix pylint

* Some docs cleanup

* Renamed iter_records -> iter_stream

* Review feedback

* Updated tests

* Missing commas

* Fix syntax

* Fix pylint

Co-authored-by: xiafu <[email protected]>

* tag sas (#12258)

* tag sas

* disable undelete_container

* pylint

* skip undelete_container tests

* [Blob][Versioning]Disable Versioning Live Test (#12281)

* [Blob][QQ]Default output_format to input_format (#12283)

* [Storage][Jumbo]Remove super (#12314)

* [Storage][JumboBlob]remove empty super()

* pypy3

* change sas version to latest

* set tags account location to central canada

* re-recording queue

* changefeed paths generator

* mark tests for vid as playback only

* fix changefeed

* fix pylint
make the test account location for tags to central canada

* add a delay before calling find_blobs_by_tags

* remove tags header

* mark a large file test playback only

* revert "mark a large file test playback only"
skip upload large file test
address comment

* move tag permission and filter_by_tags permission to kwargs

Co-authored-by: Kamil Sobol <[email protected]>
Co-authored-by: Ze Qian Zhang <[email protected]>
Co-authored-by: Rakshith Bhyravabhotla <[email protected]>
Co-authored-by: annatisch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanmcc-msft seanmcc-msft Awaiting requested review from seanmcc-msft

@zezha-msft zezha-msft Awaiting requested review from zezha-msft

@gapra-msft gapra-msft Awaiting requested review from gapra-msft

@kasobol-msft kasobol-msft Awaiting requested review from kasobol-msft

@lmazuel lmazuel Awaiting requested review from lmazuel

@amishra-dev amishra-dev Awaiting requested review from amishra-dev

@danieljurek danieljurek Awaiting requested review from danieljurek

@mayurid mayurid Awaiting requested review from mayurid

@mitchdenny mitchdenny Awaiting requested review from mitchdenny

@scbedd scbedd Awaiting requested review from scbedd

@weshaggard weshaggard Awaiting requested review from weshaggard

@annatisch annatisch Awaiting requested review from annatisch

@rakshith91 rakshith91 Awaiting requested review from rakshith91

Assignees

No one assigned

Labels

APIChange This PR contains an addition or change to the API signature and must be reviewed by an architect. Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@xiafu-msft @seanmcc-msft @rakshith91 @lmazuel @annatisch