Skip to content

Conversation

@Jg1255
Copy link
Contributor

@Jg1255 Jg1255 commented Jul 8, 2021

adding checkingpointstore for table. This is my initial pull request.

@ghost ghost added Event Hubs customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Jul 8, 2021
@ghost
Copy link

ghost commented Jul 8, 2021

Thank you for your contribution Jg1255! We will review the pull request and get back to you soon.

@swathipil swathipil removed the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Jul 8, 2021
@Jg1255 Jg1255 requested a review from scbedd as a code owner July 8, 2021 20:29
@Jg1255 Jg1255 marked this pull request as draft July 9, 2021 16:06
Copy link
Member

@swathipil swathipil left a comment

Choose a reason for hiding this comment

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

nice job with coming up with all the right methods/formatting the files! added a few comments that are Python specific, but other than that, excited to see the implementation!

Jg1255 and others added 6 commits July 9, 2021 15:28
…b/extensions/checkpointstoretable/_tablestoragecs.py

Co-authored-by: swathipil <76007337+swathipil@users.noreply.github.com>
…b/extensions/checkpointstoretable/_tablestoragecs.py

Co-authored-by: swathipil <76007337+swathipil@users.noreply.github.com>
…b/extensions/checkpointstoretable/_tablestoragecs.py

Co-authored-by: swathipil <76007337+swathipil@users.noreply.github.com>
…b/extensions/checkpointstoretable/_tablestoragecs.py

Co-authored-by: swathipil <76007337+swathipil@users.noreply.github.com>
@Jg1255 Jg1255 marked this pull request as ready for review July 12, 2021 16:22
Copy link
Contributor

@chradek chradek left a comment

Choose a reason for hiding this comment

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

Looking good, just had a few comments (mostly for @swathipil 😄)

Jg1255 and others added 2 commits July 12, 2021 17:59
…b/_version.py

Co-authored-by: swathipil <76007337+swathipil@users.noreply.github.com>
…b/extensions/checkpointstoretable/_tablestoragecs.py

Co-authored-by: swathipil <76007337+swathipil@users.noreply.github.com>
Jg1255 and others added 5 commits July 18, 2021 19:37
Co-authored-by: chradek <51000525+chradek@users.noreply.github.com>
Co-authored-by: chradek <51000525+chradek@users.noreply.github.com>
Co-authored-by: chradek <51000525+chradek@users.noreply.github.com>
@Jg1255 Jg1255 marked this pull request as draft July 19, 2021 00:09
@Jg1255 Jg1255 removed their assignment Jul 19, 2021
@Jg1255 Jg1255 marked this pull request as ready for review July 19, 2021 00:15
Copy link
Contributor

@chradek chradek left a comment

Choose a reason for hiding this comment

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

One documentation update otherwise this is looking good to me! It looks like CI is failing at the run tests step, and I am assuming that's because there aren't any tests.

@swathipil in JavaScript we'd just have a single test that made sure we were exporting the TableCheckpointStore at this point. Would you do something similar in python, or is there a way to disable the run tests step until tests are added?

Copy link
Contributor

@chradek chradek left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for going through all that work to get the initial package created!

Copy link
Member

@swathipil swathipil left a comment

Choose a reason for hiding this comment

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

hey left a couple comments but o/w LGTM!

Jg1255 and others added 2 commits July 21, 2021 12:56
Co-authored-by: swathipil <76007337+swathipil@users.noreply.github.com>
…orage_table_partition_manager.py

Co-authored-by: swathipil <76007337+swathipil@users.noreply.github.com>
@ghost
Copy link

ghost commented Jul 21, 2021

Hello @chradek!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 03f1449 into Azure:main Jul 21, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants