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

Jump to bottom

Consolidation Plan REST support - part 2: rest client #4537

Merged
merged 3 commits into from
Mar 12, 2024
Merged

Consolidation Plan REST support - part 2: rest client #4537

merged 3 commits into from
Mar 12, 2024

Conversation

ypatia
Copy link
Member

@ypatia ypatia commented Nov 24, 2023 •
edited by KiterLuc
Loading

This is part2 of #4534 , adding the rest_client part.

TYPE: FEATURE
DESC: Consolidation Plan REST support - part 2: rest client

@shortcut-integration Shortcut Integration
Copy link

This pull request has been linked to Shortcut Story #25420: [core-rest] Cloud support for consolidation plan API.

davisp
davisp previously requested changes Nov 29, 2023
View reviewed changes
Copy link
Contributor

@davisp davisp left a comment

Choose a reason for hiding this comment

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

Lets make this a draft to avoid an accidental merge.

We should also add tests for the REST CI runner. Those are conditionally enabled here:

https://github.com/TileDB-Inc/TileDB/blob/dev/test/CMakeLists.txt#L262-L267

@davisp
Copy link
Contributor

davisp commented Nov 29, 2023

I should mention, other than that, the existing changes look perfectly fine.

Base automatically changed from yt/sc-25420/rest_support_consolidation_plan to dev November 30, 2023 08:37
@ypatia ypatia marked this pull request as draft November 30, 2023 09:19
@ypatia ypatia force-pushed the yt/sc-25420/rest_support_consolidation_plan_2 branch from af6c27b to cf6ac37 Compare November 30, 2023 09:27
@ypatia
Copy link
Member Author

ypatia commented Nov 30, 2023 •
edited
Loading

Lets make this a draft to avoid an accidental merge.

We should also add tests for the REST CI runner. Those are conditionally enabled here:

https://github.com/TileDB-Inc/TileDB/blob/dev/test/CMakeLists.txt#L262-L267

Moved to Draft and added some testing for REST CI. I actually parametrized the existing test suite for c api consolidation plan, let me know if that works. I can't really test until Cloud changes are in..

@ypatia ypatia force-pushed the yt/sc-25420/rest_support_consolidation_plan_2 branch 5 times, most recently from c4a3667 to b42c77c Compare December 5, 2023 10:02
@ypatia ypatia force-pushed the yt/sc-25420/rest_support_consolidation_plan_2 branch 3 times, most recently from 394fdcd to 24f1d73 Compare December 15, 2023 14:11
@ypatia ypatia force-pushed the yt/sc-25420/rest_support_consolidation_plan_2 branch 2 times, most recently from 3db850c to e390e47 Compare January 10, 2024 14:30
@ypatia ypatia force-pushed the yt/sc-25420/rest_support_consolidation_plan_2 branch from e390e47 to aad71e4 Compare January 22, 2024 11:16
@ypatia ypatia force-pushed the yt/sc-25420/rest_support_consolidation_plan_2 branch from aad71e4 to 62be988 Compare February 23, 2024 11:40
@ypatia ypatia marked this pull request as ready for review February 23, 2024 11:48
@ypatia ypatia marked this pull request as draft February 23, 2024 12:57
@ypatia ypatia force-pushed the yt/sc-25420/rest_support_consolidation_plan_2 branch from 62be988 to 311f514 Compare March 6, 2024 14:22
@ypatia ypatia marked this pull request as ready for review March 6, 2024 14:22
@KiterLuc KiterLuc dismissed davisp’s stale review March 12, 2024 12:55

Checked with Paul, we should be good here.

@ypatia ypatia force-pushed the yt/sc-25420/rest_support_consolidation_plan_2 branch from 311f514 to 4a9ff50 Compare March 12, 2024 13:14
@KiterLuc KiterLuc merged commit a0e82bd into dev Mar 12, 2024
58 of 59 checks passed
@KiterLuc KiterLuc deleted the yt/sc-25420/rest_support_consolidation_plan_2 branch March 12, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KiterLuc KiterLuc KiterLuc approved these changes

@davisp davisp davisp left review comments

@anastasop anastasop Awaiting requested review from anastasop

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ypatia @davisp @KiterLuc