Skip to content

Control plane tests (batch 3) #274

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
merged 5 commits into from
Feb 19, 2025
Merged

Control plane tests (batch 3) #274

merged 5 commits into from
Feb 19, 2025
+320 −91

Conversation

@osipovartem
Copy link
Contributor

@osipovartem osipovartem commented Feb 18, 2025 •
edited
Loading

@eadgbear
Copy link
Contributor

eadgbear commented Feb 18, 2025

Instead of mocking the S3 client, we can explore using Bollard to start up a Minio docker container.

@osipovartem
Copy link
Contributor Author

osipovartem commented Feb 18, 2025

Instead of mocking the S3 client, we can explore using Bollard to start up a Minio docker container.

This should be added as a part of integration tests, let's add unit tests first. We can add a separate workflow or mark some tests with specific tag to run them as integration. WDYT?

@eadgbear
Copy link
Contributor

eadgbear commented Feb 18, 2025

Instead of mocking the S3 client, we can explore using Bollard to start up a Minio docker container.

This should be added as a part of integration tests, let's add unit tests first. We can add a separate workflow or mark some tests with specific tag to run them as integration. WDYT?

Fair. I just don't think that mocking an S3 client is necessary outside of Integration tests. Unit test should ideally be Memory only, then integration tests for File/S3-based tests, IMO

rampage644
rampage644 reviewed Feb 18, 2025
View reviewed changes
@osipovartem
Copy link
Contributor Author

osipovartem commented Feb 19, 2025

Instead of mocking the S3 client, we can explore using Bollard to start up a Minio docker container.

This should be added as a part of integration tests, let's add unit tests first. We can add a separate workflow or mark some tests with specific tag to run them as integration. WDYT?

Fair. I just don't think that mocking an S3 client is necessary outside of Integration tests. Unit test should ideally be Memory only, then integration tests for File/S3-based tests, IMO

That's why I added a mock to not use any real s3 client calls. This mock works in memory only and it allows us to cover create_storage_profile and validate_credentials endpoints by unit tests without any external calls

@osipovartem osipovartem merged commit 8bb4b74 into main Feb 19, 2025
4 of 5 checks passed
@osipovartem osipovartem deleted the issues/264_test_coverage branch February 19, 2025 18:38
eadgbear pushed a commit that referenced this pull request Feb 27, 2025
@osipovartem @eadgbear
Control plane tests (batch 3) (#274)
8ffaf52 
* Add CP service tests

* Add create/delete session tests

* Add create/delete session tests

* Use converted S3 error
@eadgbear eadgbear mentioned this pull request Feb 27, 2025
Merged
eadgbear pushed a commit that referenced this pull request Mar 6, 2025
@osipovartem @eadgbear
Control plane tests (batch 3) (#274)
6b6719b 
* Add CP service tests

* Add create/delete session tests

* Add create/delete session tests

* Use converted S3 error
eadgbear added a commit that referenced this pull request Mar 7, 2025
@eadgbear @osipovartem
@YaroslavLitvinov @Vedin @rampage644 @DanCodedThis
(WIP) Metastore + Unified Query 1.0 (#312)
70793f5 
* Initial metastore crate

* Metastore implementation

* Upgrading Slatedb to 0.4, adding snapshot tests

* Checkpoint commit

* Checkpoint before crate re-org

* Fix timestamp displaying format (#279)

* fix timestamp format

* Revert timestamp change

* in login request use database_name instead of warehouse (#277)

* Adding missing licenses + license check (#278)

* Adding missing licenses + license check

* Fixing license config path

* Fix IPC arrow format issue (#280)

* Fix IPC arrow format issue

* Fix IPC arrow format issue

* Fix comments

* Control plane tests (batch 3) (#274)

* Add CP service tests

* Add create/delete session tests

* Add create/delete session tests

* Use converted S3 error

* Move DBT_SERIALIZATION_FORMAT to cli (#286)

* Move dbt_ser_fmt to cli

* Rename the field

* Update query runner with session id (#285)

* Issue 115 Arrow support: replace uints by ints in resulted RecordBatch (#287)

* replace uints by ints in resulted RecordBatch

* cargo clippy --workspace --tests

* rebase & fixes

* Set warehouse location based on storage profile path (#288)

* map float types to real logical_type (#289)

* map float types to real logical_type

* separate sessions tests to avoid blocking issues

* unit test for floats datatype maped into real logical type

* Snowflake timestamp_from_parts udf (#297)

* Inital version

* snowflake timestamp_from_parts udf

* Fix clippy

* Fix example typo

* Support negative values

* Snowflake time_from_parts udf (#298)

* Inital version

* snowflake timestamp_from_parts udf

* snowflake time_from_parts udf

* snowflake time_from_parts udf

* Fix clippy

* Fix clippy

* Fix example typo

* Support negative values

* Merge

* snowflake date_from_parts udf (#299)

* Inital version

* snowflake timestamp_from_parts udf

* snowflake time_from_parts udf

* snowflake time_from_parts udf

* snowflake date_from_parts udf

* Fix clippy

* Implement extraction of the corresponding date part from a date or timestamp (#302)

* Implement extracts the corresponding date part from a date or timestamp

* Add units for visit expressions

* Add geospatial udfs (#304)

* Fix object store for custom endpoint (#295)

AWS S3 builder uses dedicated setting to allow http. Parse endpoint and enable if it isn't https.

* Unified query, execution, catalog

* Adding temporary tables to metastore

* Custom DataFusion Catalog implementation (WIP)

* Adding execution User Session type

* Checkpoint

* Catalog implementation for IB

* Crate re-org complete, writing tests

* Changing metastore crate to use iceberg_rust types

* Metastore crate cleanup

* Create table query fixed

* Fixing copy_into query

* Finished updating query type

* Cleaning up service type

* Wrapping up changes to the execution crate

* Bumping Datafusion git ref

* Fixing up after package reorg

* Fixed DBT handler

* Removing old crates

* Disabling some UI handlers and models until new metastore integration is done

* Disabling extra OpenAPI endpoints and some unit tests while test are fixed

* Deleting manifest files that snuck into the repo (my bad)

* Final compilation and clippy fixes

* `cargo fix`

* `cargo fmt`

* Adding license blocks to new files

* Adding missing licenses + license check (#278)

* Adding missing licenses + license check

* Fixing license config path

* Fix IPC arrow format issue (#280)

* Fix IPC arrow format issue

* Fix IPC arrow format issue

* Fix comments

* Control plane tests (batch 3) (#274)

* Add CP service tests

* Add create/delete session tests

* Add create/delete session tests

* Use converted S3 error

* Move DBT_SERIALIZATION_FORMAT to cli (#286)

* Move dbt_ser_fmt to cli

* Rename the field

* Snowflake timestamp_from_parts udf (#297)

* Inital version

* snowflake timestamp_from_parts udf

* Fix clippy

* Fix example typo

* Support negative values

* Implement extraction of the corresponding date part from a date or timestamp (#302)

* Implement extracts the corresponding date part from a date or timestamp

* Add units for visit expressions

* Add geospatial udfs (#304)

* patch chrono 0.4.40 as it breaks build (#310)

* Added support for `day` and other datetime keywords in DF (#294)

* start here

* no macro, just simple vistor mut

* remvoe redundant trait creation

* integration test add, fixed logic, next unit tests

* activated impl and cargo fmt + clippy

* added unit tests + impl extension

* cargo fmt + clippy

* cargo clippy

* cargo clippy

* removed redundant check

* fix

* merged with Artem PR

* cargo fmt + clippy

* small fixes

* cargo fmt + clippy

* update hash commis for datafusion, iceberg-rust (#301)

* Update tests.yml (#313)

Adding github large runners to tests

* Return hardcoded is_dynamic (#318)

* Add geospatial UDF st_makeline (#315)

* Add geospatial udfs

* Add geospatial st_makeline

* Fix cargo

* Add lic header

* Change allocator to snmalloc (#317)

* Add ST_MakePolygon, st_polygon (#319)

* Add ST_MakePolygon, st_polygon

* Add docs

* Add ST_DIMENSION  (#321)

* Add ST_MakePolygon, st_polygon

* Add docs

* Add ST_dimention

* Add license

* Add ST_PointN and ST_Endpoint (#322)

* Add ST_MakePolygon, st_polygon

* Add docs

* Add ST_dimention

* Add license

* Add ST_Endpoint, ST_PointN

* Reimplement ST_Point to work with snowflake linestring

* Add St_x, st_y, st_srid (#324)

* Add ST_MakePolygon, st_polygon

* Add docs

* Add ST_dimention

* Add license

* Add ST_Endpoint, ST_PointN

* Reimplement ST_Point to work with snowflake linestring

* Add St_x, st_y, st_srid

* Moving UDF registration

* Changing Metastore to use Dashmap

* Adding missing geospatial functions after merge

* Fixing unit test structure, still need to fix some unit tests

* Post-merge cleanup

---------

Co-authored-by: Artem Osipov <[email protected]>
Co-authored-by: Yaroslav Litvinov <[email protected]>
Co-authored-by: Denys Tsomenko <[email protected]>
Co-authored-by: Sergei Turukin <[email protected]>
Co-authored-by: DanCodedThis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rampage644 rampage644 rampage644 approved these changes

@DanCodedThis DanCodedThis DanCodedThis approved these changes

@eadgbear eadgbear Awaiting requested review from eadgbear

@YaroslavLitvinov YaroslavLitvinov Awaiting requested review from YaroslavLitvinov

@Vedin Vedin Awaiting requested review from Vedin

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@osipovartem @eadgbear @rampage644 @DanCodedThis