-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
add implementation for checkpointstoretable #19905
Conversation
Thank you for your contribution Jg1255! We will review the pull request and get back to you soon. |
9be9b52
to
2f974c4
Compare
add the test cases add the test file fix test file
2f974c4
to
a6875dd
Compare
/azp run python - eventhub - ci |
Azure Pipelines successfully started running 1 pipeline(s). |
...nthub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py
Outdated
Show resolved
Hide resolved
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…sdk-for-python into josue_third_branch
...nthub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py
Outdated
Show resolved
Hide resolved
...nthub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py
Outdated
Show resolved
Hide resolved
...nthub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py
Outdated
Show resolved
Hide resolved
...nthub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py
Outdated
Show resolved
Hide resolved
mode=UpdateMode.REPLACE, | ||
entity=ownership_entity, | ||
etag=ownership["etag"], | ||
match_condition=MatchConditions.IfNotModified, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, now that I think about it, why are we setting this as the MatchCondition? is this what Baffy did in JS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove this, b/c it implies that we only update the entity if it has not been modified before. However, if we call this method twice (or more) on the same entity, it should update it twice (or more).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we actually need it cause without the match condition it will not update the owner_id and raise an exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's weird, but since we're not releasing this rn, I can check if this is the right approach later
...nthub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py
Outdated
Show resolved
Hide resolved
sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py
Show resolved
Hide resolved
sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py
Show resolved
Hide resolved
sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py
Show resolved
Hide resolved
aiohttp>=3.0; python_version >= '3.5' | ||
../../core/azure-core | ||
../azure-eventhub | ||
../../tables/azure-data-tables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldn't be touching this file at all since this is actually a management plane library. We on the data plane team are not in charge of this library. Let's undo these changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so undoing the changes would mean adding this line again:
aiohttp>=3.0; python_version >= '3.5'
and removing these:
../../core/azure-core
../azure-eventhub
../../tables/azure-data-tables
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…orage_table_partition_manager.py Co-authored-by: swathipil <[email protected]>
…orage_table_partition_manager.py Co-authored-by: swathipil <[email protected]>
…orage_table_partition_manager.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
…b/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]>
/azp run python - eventhub - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, left a few last comments 🤞
sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py
Show resolved
Hide resolved
sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py
Show resolved
Hide resolved
sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py
Show resolved
Hide resolved
sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py
Show resolved
Hide resolved
sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py
Show resolved
Hide resolved
sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py
Show resolved
Hide resolved
aiohttp>=3.0; python_version >= '3.5' | ||
../../core/azure-core | ||
../azure-eventhub | ||
../../tables/azure-data-tables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so undoing the changes would mean adding this line again:
aiohttp>=3.0; python_version >= '3.5'
and removing these:
../../core/azure-core
../azure-eventhub
../../tables/azure-data-tables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for getting this implementation put together! And thanks @swathipil for reviewing!
…into have_pipelines_support_rest * 'main' of https://github.com/Azure/azure-sdk-for-python: (108 commits) Enable API review approval check for Java spring packages (Azure#20311) [AutoRelease] t2-iothubprovisioningservices-2021-07-15-81882 (Azure#19816) Increment package version after release of azure-eventgrid (Azure#20204) Sync eng/common directory with azure-sdk-tools for PR 1909 (Azure#20298) Rename attrs is metrics (Azure#20236) [EventHubs] checkpointstoretable - skip tests until env vars configured (Azure#20289) Update to use timespan (Azure#20233) Prevent wildcard expansion in git sparse checkout add (Azure#20267) Fix typo and polish the key concepts (Azure#18407) Fix IOT Device Update readme issue (Azure#18752) Add context manager API to azure.identity credentials (Azure#19746) Add support for 'files' configuration (Azure#20272) Update main for 30-close.py (Azure#20287) [AutoRelease] t2-purview-2021-08-13-30358 (Azure#20260) add implementation for checkpointstoretable (Azure#19905) Fix resource clean-up script (Azure#20273) [Tables] Fix bug in update mode (Azure#20264) Add Rest Method checks to Prepare-Release (Azure#20275) Update release date (Azure#20270) Update the release date for ACS chat 1.1.0b1 (Azure#20277) ...
* add implementation add the test cases add the test file fix test file * fix pylint * fix pylint * fix pylint * fix pylint * fix pylint * fix pylint * fix pylint * fix pylint * fix pylint * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * new update * Update shared_requirements.txt Co-authored-by: swathipil <[email protected]> * environment * redo * redo * redo * changed variable name * changed variable name * Update sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py Co-authored-by: Sean Kane <[email protected]> * update on test file * update on test file * update on test file * update based on feedack * new update * update * update * update * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: chradek <[email protected]> * new update * new * new * new * new * new * update on spacing * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: chradek <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: chradek <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: chradek <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: chradek <[email protected]> * new update * update * update on test file * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * update * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * update * update * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * update * update * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * update * Update sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py Co-authored-by: swathipil <[email protected]> * update * Update sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py Co-authored-by: swathipil <[email protected]> * update * update * update * update * update * update * Revert "update" This reverts commit d2dbb2a. * update * update * update * newupdate * update * update * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * update * update * update * update * update * update * update * update * update * update * update * update * update * update * update * update * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/samples/receive_events_using_checkpoint_store.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/setup.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py Co-authored-by: swathipil <[email protected]> * update * update * update * update * update * update * p * update * update * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/tests/test_storage_table_partition_manager.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * Update sdk/eventhub/azure-eventhub-checkpointstoretable/azure/eventhub/extensions/checkpointstoretable/_tablestoragecs.py Co-authored-by: swathipil <[email protected]> * update * update * update Co-authored-by: Josue Garcia <[email protected]> Co-authored-by: swathipil <[email protected]> Co-authored-by: Sean Kane <[email protected]> Co-authored-by: chradek <[email protected]>
implementation changes made to checkpointstoretable