[Event Hubs] Port azure storage blob code into checkpointstore #9950
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
added 6 commits
February 14, 2020 11:23
YijunXieMS
requested review from
lmazuel,
mayurid,
annatisch,
xiangyan99,
johanste,
KieranBrantnerMagee and
yunhaoling
lmazuel
reviewed
Feb 24, 2020
added 7 commits
February 24, 2020 15:36
|
/azp run python - eventhubs - ci |
|
Pull request contains merge conflicts. |
|
/azp run python - eventhubs - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
annatisch
reviewed
Mar 4, 2020
| @@ -97,6 +97,20 @@ if __name__ == '__main__': | |||
| loop.run_until_complete(main()) | |||
| ``` | |||
|
|
|||
| #### Use `BlobCheckpointStore` with a different version of Azure Storage Service API | |||
| Some environments have different versions of Azure Storage Service API. For instance, some Azure Stack use 2017-11-09. | |||
There was a problem hiding this comment.
"some Azure Stack deployments"?
There was a problem hiding this comment.
Not sure if we have other deployments that are not Azure Stack
There was a problem hiding this comment.
In that case I would change it to "For instance, Azure Stack may use...."
...heckpointstoreblob-aio/azure/eventhub/extensions/checkpointstoreblobaio/_blobstoragecsaio.py
Show resolved
Hide resolved
annatisch
reviewed
Mar 5, 2020
| if headers: | ||
| headers["x-ms-version"] = api_version | ||
| else: | ||
| kwargs["headers"] = {"x-ms-version": api_version} |
There was a problem hiding this comment.
You could simplify:
headers = kwargs.get("headers") or {}
headers["x-ms-version"] = api_version
annatisch
reviewed
Mar 5, 2020
| loop = asyncio.get_event_loop() | ||
| checkpoint_store = BlobCheckpointStore.from_connection_string( | ||
| STORAGE_CONNECTION_STR, | ||
| container_name=BLOB_CONTAINER_NAME, |
There was a problem hiding this comment.
API version sample doesn't use API version :)
annatisch
reviewed
Mar 5, 2020
| @@ -2,6 +2,9 @@ | |||
|
|
|||
| ## 1.0.1 (Unreleased) | |||
|
|
|||
| **New features** | |||
| - Param `api_version` of `BlobPartitionManager` now supports older versions of Azure Storage Serice API. | |||
There was a problem hiding this comment.
Fixed. good catch
annatisch
previously approved these changes
Mar 5, 2020
annatisch
approved these changes
Mar 5, 2020
|
/azp run python - eventhubs - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
This PR is for issue #7808 - Remove checkpointstore's dependency on azure blob storage.
An example is the latest azure-storage-blob SDK v12.x supports storage service API 2019-02-02 and 2019-07-07. But Azure Stack's storage service API version is 2017-11-09. So lib azure-eventhub-checkpointstoreblob and azure-eventhub-checkpointblob-aio can't work on Azure Stack.
This PR is to port the latest storage blob SDK code to the two checkpointstore libs and make it to be compatible with storage 2017-11-09. And this doesn't introduce a break change to the user.
These changes are made:
An Azure Stack user will need to specify storage service api_version when creating an BlobCheckpointStore if the api_version isn't the latest version.