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.

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

kedro-datasets: allow skipping Python 3.12 unit tests for SnowflakeTableDatasets #890

Closed
tdhooghe opened this issue Oct 15, 2024 · 5 comments

Comments

@tdhooghe
Copy link
Contributor

tdhooghe commented Oct 15, 2024

Description

Currently, installing testing requirements are failing for SnowflakeTableDatasets for Python 3.12, because it is not supported yet by the SnowparkAPI... Skipping Python 3.12 makes the tests succeed, as shown here

Context

I want to create a PR (#881) for SnowflakeTableDatasets, and the unit testing is failing.

Possible Implementation

Update CI/CD in such a way that it can skip unit tests for Python 3.12

Possible Alternatives

  • Skipping unit tests for SnowflakeTablesets like was done in the past, because of poor previous compatibility of SnowparkAPI.
  • Praying for Snowflake to finally update Snowpark to be compatible with Python 3.12.
@merelcht
Copy link
Member

Hi @tdhooghe, thanks for opening this issue. kedro-plugins has a pretty complicated setup and the only way I think we've managed to skip unit tests for a specific dataset in the past is by using Makefile commands and excluding it from the default job. However that means that the tests aren't run automatically.

We're more than happy to accept a contribution to make this work if you have an idea!

@tdhooghe
Copy link
Contributor Author

tdhooghe commented Oct 15, 2024

Hi @merelcht, thank you for your response :)

Currently, SnowflakeTableDatasets are not included in testing (see here), because it needs a dedicated instance of Snowflake running (see here).

Happy to keep it this way and merge #881, and bumping the snowflake-snowpark-python version to 1.23. That would be already much better than the current implementation of SnowflakeTableDataset which is stuck at version 1.0.0 (that only supports the depreciated Python 3.8 version). What do you think of this?

In the future, I would also be happy to include this testing framework, instead of the way it is currently setup.

@merelcht
Copy link
Member

Hi @tdhooghe that sounds good to me. I was actually thinking we probably need to demote this dataset to experimental, since it doesn't technically adhere to our requirements of testing and python version support.

Of course, if the testing framework you linked works fine, we can then make it a core dataset again.

@tdhooghe
Copy link
Contributor Author

Hmm makes sense, although I think it would be really nice to have this one as core dataset.. If I implement the testing framework above, would the team then be fine of not having compatibility with Python 3.12 (for now, and once it is there, I will make the PR for including)?

@merelcht
Copy link
Member

Done in #881

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants