-
Notifications
You must be signed in to change notification settings - Fork 98
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
ci: port lint, unit test, and e2e tests to Actions #155
Conversation
Signed-off-by: Ankita Katiyar <[email protected]>
Signed-off-by: Ankita Katiyar <[email protected]>
FYI @SajidAlamQB and @AhdraMeraliQB |
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.
Few questions:
- Are the unit tests triggered for all dataset? Can we trigger it conditionally with file changes like the current setup?
- for the secret-scan, is there a way to disable by line level instead of file level? (similar to
pylint: disable=xxx
- Not entirely sure why lint fails only 3.8, is that because of missing imports? Can you share the related CI run.
[Question] Adding macOS test to this would be fairly trivial - just adding macos-latest to matrix.os list in the unit-tests job. Should we do this? The Github action workflows are slow mostly for kedro-datasets (at least the way it's set up now) but they run in parallel.
I'd love to see them but exclude it from our merge requirements.
[Question] Should we also remove the corresponding CircleCI workflows as a part of the same PR?
No strong opinion about this, we can keep it run in parallel for a sprint if we want to take the safe side as long as it's not slowing us down. (I don't think it will anyway)
Haven't look into the config in details, so I just have high-level questions. I will have another look at this again.
Thanks @noklam for taking a look.
The current setup with circleCI also runs the tests for all datasets if the changes are made in
I couldn't find a way to disable it for that specific line.
Lint test currently on circleCi also is only set up to run with python 3.8. For any other version
I'll add it. As a side note, I think none of the tests currently are required to pass before merging, only two approvals are required. This is part of the repo settings not CI config. |
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.
Is it necessary to have a separate CI file for each plugin? The content is pretty much the same apart from the specific plugin name, so I'm wondering if it's possible to generalise and just trigger the right plugin build depending on the changes made?
If that's not straightforward, I'd be happy to get it setup like this first, and then we can try optimise later 🙂
Thanks @merelcht! |
Path filtering is very much possible, I can help with that later. |
Update: Thanks to @SajidAlamQB for getting the end to end tests running! 🎉 I've also added them to this PR! |
|
I'm dumb/haven't had my coffee; this is the existing CircleCI. 🤦 Well, glad the move to GitHub Actions fixes that. |
.github/workflows/unit-test.yml
Outdated
pip install git+https://github.com/kedro-org/kedro@main | ||
pip install -r test_requirements.txt |
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.
pip install git+https://github.com/kedro-org/kedro@main | |
pip install -r test_requirements.txt | |
pip install git+https://github.com/kedro-org/kedro@main -r test_requirements.txt |
Didn't check if this is also there is the existing CI, but we should not have multiple pip install
commands, because they can override dependencies installed in a previous step.
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.
The current CI setup does it this way as well, kedro is installed first and then the test_requirements.txt
for the plugin being tested. I tried making this change in my forked repo but the tests start failing at the "Installing dependencies" stage for kedro-datasets
because of dependency resolution conflict. (See this failed run)
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 this is a critical problem that needs resolving--whether it's in scope of this PR or not is a separate issue.
What this means is that, in reality, we don't have resolvable requirements, and we're only able to get to a resolvable state by overwriting some of the previously-installed requirements. Some of the stuff installed in the pip install -r test_requirements.txt
are not actually going to be compatible with Kedro on main
, it seems.
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.
This is definitely something we need to look into more. I'd suggest creating a separate ticket.
.github/workflows/unit-test.yml
Outdated
pip install git+https://github.com/kedro-org/kedro@main | ||
pip install -r test_requirements.txt |
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.
Same
pip install git+https://github.com/kedro-org/kedro@main | |
pip install -r test_requirements.txt | |
pip install git+https://github.com/kedro-org/kedro@main -r test_requirements.txt |
.github/workflows/unit-test.yml
Outdated
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v3 | ||
- name: Set up Python 3.8 |
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.
Nit: Any reason not to lint on the latest supported version instead of 3.8?
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.
Current setup also runs lint on 3.8. The lint tests fail for other versions of python for kedro-datasets
because of this snowflake-snowpark-python
is only installed for python version 3.8 so pylint throws import errors. We can change the python version to 3.10 and suppress the error
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.
Thanks for answering, I'm fine with either route (or doing this in a separate task in the future); I was just wondering. :)
Sorry for haphazardly dropping comments; I'm sure I've violated all sorts of PR review etiquette! Looks great overall. :) Left a number of comments, but I think a few that are particularly important to address (IMO):
|
Signed-off-by: Ankita Katiyar <[email protected]>
Signed-off-by: Ankita Katiyar <[email protected]>
Signed-off-by: Ankita Katiyar <[email protected]>
Signed-off-by: Ankita Katiyar <[email protected]>
@ankatiyar To answer your questions on the PR description:
What is the difference between the
I would do this in a separate PR to make it easier to review. It would be nice if you could do a small show and tell for the team to talk about what's different and how we access the builds etc 🙂 |
Signed-off-by: Ankita Katiyar <[email protected]>
Update: I've been able make the windows tests work by modifying the @merelcht There is no difference between the setup for |
If there's no difference I don't really see a point in adding them to be honest. |
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.
Great work @ankatiyar and also @SajidAlamQB for making the Windows tests work 👍
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.
This is awesome work @ankatiyar! 🌟 I believe this is in a good state for now. We can optimise things like pip install
overwrites in subsequent issues.
Signed-off-by: Ankita Katiyar <[email protected]>
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 overall! Added some comments, but I think the main thing that would be quick/easy to try and add some value is adding pytables
to the requirements file with an environment marker. (Also, if you could update the PR title to not be Linux specific now, or I can do it.)
- name: Cache python packages for Linux | ||
if: matrix.os == 'ubuntu-latest' | ||
uses: actions/cache@v3 | ||
with: | ||
path: ~/.cache/pip | ||
key: ${{inputs.plugin}}-${{matrix.os}}-python-${{matrix.python-version}} | ||
restore-keys: ${{inputs.plugin}} | ||
- name: Cache python packages for Windows | ||
if: matrix.os == 'windows-latest' | ||
uses: actions/cache@v3 | ||
with: | ||
path: ~\AppData\Local\pip\Cache | ||
key: ${{inputs.plugin}}-${{matrix.os}}-python-${{matrix.python-version}} | ||
restore-keys: ${{inputs.plugin}} |
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.
https://github.com/actions/cache/blob/main/tips-and-workarounds.md#cross-os-cache looks cool, but not sure it would work
- name: Install pytables (only for kedro-datasets on windows) | ||
if: matrix.os == 'windows-latest' && inputs.plugin == 'kedro-datasets' | ||
run: pip install 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.
Just thinking about it--if using pip, should this not be part of the test_requirements.txt
, with an environment marker (like https://stackoverflow.com/a/54281345)
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 quickly tried it. The test_requirements.txt
for kedro-datasets
has this -
tables~=3.6.0; platform_system == "Windows" and python_version < '3.9'
tables~=3.6; platform_system != "Windows"
installingtables 3.6
does not work for windows with python 3.10. I can check in a separate PR if the version of pyTables can be safely bumped and this step can be removed after this is merged in.
- name: Run unit tests for Windows / kedro-airflow, kedro-docker, kedro-telemetry | ||
if: matrix.os == 'windows-latest' && inputs.plugin != 'kedro-datasets' | ||
run: | | ||
cd ${{ inputs.plugin }} | ||
pytest tests | ||
- name: Run unit tests for Windows / kedro-datasets / no spark sequential | ||
if: matrix.os == 'windows-latest' && inputs.plugin == 'kedro-datasets' && matrix.python-version == '3.10' | ||
run: | | ||
make test-no-spark-sequential | ||
- name: Run unit tests for Windows / kedro-datasets / no spark parallel | ||
if: matrix.os == 'windows-latest' && inputs.plugin == 'kedro-datasets' && matrix.python-version != '3.10' | ||
run: | | ||
make test-no-spark |
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.
Maybe an item outside of the scope of the PR, but is this the same behavior on the current CircleCI build? Seems so convoluted. Maybe it makes more sense to abstract most (all?) of these differences in the Makefile, if using one anyway.
.github/workflows/check-plugin.yml
Outdated
cd ${{ inputs.plugin }} | ||
pip install git+https://github.com/kedro-org/kedro@main | ||
pip install -r test_requirements.txt | ||
pip freeze |
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.
Nit: You have pip freeze
as a separate step above, and as part of requirements installation in this workflow.
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.
Moved this to a separate step here as well.
Signed-off-by: Ankita Katiyar <[email protected]>
Signed-off-by: Ankita Katiyar <[email protected]>
This reverts commit 8203daa.
Signed-off-by: Ankita Katiyar <[email protected]>
* Add unit test + lint test on GA * trigger GA - will revert Signed-off-by: Ankita Katiyar <[email protected]> * Fix lint Signed-off-by: Ankita Katiyar <[email protected]> * Add end to end tests * Add cache key Signed-off-by: Ankita Katiyar <[email protected]> * Add cache action Signed-off-by: Ankita Katiyar <[email protected]> * Rename workflow files Signed-off-by: Ankita Katiyar <[email protected]> * Lint + add comment + default bash Signed-off-by: Ankita Katiyar <[email protected]> * Add windows test Signed-off-by: Ankita Katiyar <[email protected]> * Update workflow name + revert changes to READMEs Signed-off-by: Ankita Katiyar <[email protected]> * Add kedro-telemetry/RELEASE.md to trufflehog ignore Signed-off-by: Ankita Katiyar <[email protected]> * Add pytables to test_requirements remove from workflow Signed-off-by: Ankita Katiyar <[email protected]> * Revert "Add pytables to test_requirements remove from workflow" This reverts commit 8203daa. * Separate pip freeze step Signed-off-by: Ankita Katiyar <[email protected]> --------- Signed-off-by: Ankita Katiyar <[email protected]> Signed-off-by: Tingting_Wan <[email protected]>
* Add unit test + lint test on GA * trigger GA - will revert Signed-off-by: Ankita Katiyar <[email protected]> * Fix lint Signed-off-by: Ankita Katiyar <[email protected]> * Add end to end tests * Add cache key Signed-off-by: Ankita Katiyar <[email protected]> * Add cache action Signed-off-by: Ankita Katiyar <[email protected]> * Rename workflow files Signed-off-by: Ankita Katiyar <[email protected]> * Lint + add comment + default bash Signed-off-by: Ankita Katiyar <[email protected]> * Add windows test Signed-off-by: Ankita Katiyar <[email protected]> * Update workflow name + revert changes to READMEs Signed-off-by: Ankita Katiyar <[email protected]> * Add kedro-telemetry/RELEASE.md to trufflehog ignore Signed-off-by: Ankita Katiyar <[email protected]> * Add pytables to test_requirements remove from workflow Signed-off-by: Ankita Katiyar <[email protected]> * Revert "Add pytables to test_requirements remove from workflow" This reverts commit 8203daa. * Separate pip freeze step Signed-off-by: Ankita Katiyar <[email protected]> --------- Signed-off-by: Ankita Katiyar <[email protected]> Signed-off-by: Danny Farah <[email protected]>
* Add unit test + lint test on GA * trigger GA - will revert Signed-off-by: Ankita Katiyar <[email protected]> * Fix lint Signed-off-by: Ankita Katiyar <[email protected]> * Add end to end tests * Add cache key Signed-off-by: Ankita Katiyar <[email protected]> * Add cache action Signed-off-by: Ankita Katiyar <[email protected]> * Rename workflow files Signed-off-by: Ankita Katiyar <[email protected]> * Lint + add comment + default bash Signed-off-by: Ankita Katiyar <[email protected]> * Add windows test Signed-off-by: Ankita Katiyar <[email protected]> * Update workflow name + revert changes to READMEs Signed-off-by: Ankita Katiyar <[email protected]> * Add kedro-telemetry/RELEASE.md to trufflehog ignore Signed-off-by: Ankita Katiyar <[email protected]> * Add pytables to test_requirements remove from workflow Signed-off-by: Ankita Katiyar <[email protected]> * Revert "Add pytables to test_requirements remove from workflow" This reverts commit 8203daa. * Separate pip freeze step Signed-off-by: Ankita Katiyar <[email protected]> --------- Signed-off-by: Ankita Katiyar <[email protected]> Signed-off-by: jmcdonnell <[email protected]>
* Fix links on GitHub issue templates (#150) Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * add spark_stream_dataset.py Signed-off-by: Tingting_Wan <[email protected]> * Migrate most of `kedro-datasets` metadata to `pyproject.toml` (#161) * Include missing requirements files in sdist Fix gh-86. Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Migrate most project metadata to `pyproject.toml` See kedro-org/kedro#2334. Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Move requirements to `pyproject.toml` Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> --------- Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * restructure the strean dataset to align with the other spark dataset Signed-off-by: Tingting_Wan <[email protected]> * adding README.md for specification Signed-off-by: Tingting_Wan <[email protected]> * Update kedro-datasets/kedro_datasets/spark/spark_stream_dataset.py Co-authored-by: Nok Lam Chan <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * rename the dataset Signed-off-by: Tingting_Wan <[email protected]> * resolve comments Signed-off-by: Tingting_Wan <[email protected]> * fix format and pylint Signed-off-by: Tingting_Wan <[email protected]> * Update kedro-datasets/kedro_datasets/spark/README.md Co-authored-by: Deepyaman Datta <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * add unit tests and SparkStreamingDataset in init.py Signed-off-by: Tingting_Wan <[email protected]> * add unit tests Signed-off-by: Tingting_Wan <[email protected]> * update test_save Signed-off-by: Tingting_Wan <[email protected]> * Upgrade Polars (#171) * Upgrade Polars Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Update Polars to 0.17.x --------- Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * if release is failed, it return exit code and fail the CI (#158) Signed-off-by: Tingting_Wan <[email protected]> * Migrate `kedro-airflow` to static metadata (#172) * Migrate kedro-airflow to static metadata See kedro-org/kedro#2334. Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Add explicit PEP 518 build requirements for kedro-datasets Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Typos Co-authored-by: Merel Theisen <[email protected]> Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Remove dangling reference to requirements.txt Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Add release notes Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> --------- Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * Migrate `kedro-telemetry` to static metadata (#174) * Migrate kedro-telemetry to static metadata See kedro-org/kedro#2334. Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Add release notes Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> --------- Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * ci: port lint, unit test, and e2e tests to Actions (#155) * Add unit test + lint test on GA * trigger GA - will revert Signed-off-by: Ankita Katiyar <[email protected]> * Fix lint Signed-off-by: Ankita Katiyar <[email protected]> * Add end to end tests * Add cache key Signed-off-by: Ankita Katiyar <[email protected]> * Add cache action Signed-off-by: Ankita Katiyar <[email protected]> * Rename workflow files Signed-off-by: Ankita Katiyar <[email protected]> * Lint + add comment + default bash Signed-off-by: Ankita Katiyar <[email protected]> * Add windows test Signed-off-by: Ankita Katiyar <[email protected]> * Update workflow name + revert changes to READMEs Signed-off-by: Ankita Katiyar <[email protected]> * Add kedro-telemetry/RELEASE.md to trufflehog ignore Signed-off-by: Ankita Katiyar <[email protected]> * Add pytables to test_requirements remove from workflow Signed-off-by: Ankita Katiyar <[email protected]> * Revert "Add pytables to test_requirements remove from workflow" This reverts commit 8203daa. * Separate pip freeze step Signed-off-by: Ankita Katiyar <[email protected]> --------- Signed-off-by: Ankita Katiyar <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * Migrate `kedro-docker` to static metadata (#173) * Migrate kedro-docker to static metadata See kedro-org/kedro#2334. Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Address packaging warning Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Fix tests Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Actually install current plugin with dependencies Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Add release notes Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> --------- Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * Introdcuing .gitpod.yml to kedro-plugins (#185) Currently opening gitpod will installed a Python 3.11 which breaks everything because we don't support it set. This PR introduce a simple .gitpod.yml to get it started. Signed-off-by: Tingting_Wan <[email protected]> * sync APIDataSet from kedro's `develop` (#184) * Update APIDataSet Signed-off-by: Nok Chan <[email protected]> * Sync ParquetDataSet Signed-off-by: Nok Chan <[email protected]> * Sync Test Signed-off-by: Nok Chan <[email protected]> * Linting Signed-off-by: Nok Chan <[email protected]> * Revert Unnecessary ParquetDataSet Changes Signed-off-by: Nok Chan <[email protected]> * Sync release notes Signed-off-by: Nok Chan <[email protected]> --------- Signed-off-by: Nok Chan <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * formatting Signed-off-by: Tingting_Wan <[email protected]> * formatting Signed-off-by: Tingting_Wan <[email protected]> * formatting Signed-off-by: Tingting_Wan <[email protected]> * formatting Signed-off-by: Tingting_Wan <[email protected]> * add spark_stream_dataset.py Signed-off-by: Tingting_Wan <[email protected]> * restructure the strean dataset to align with the other spark dataset Signed-off-by: Tingting_Wan <[email protected]> * adding README.md for specification Signed-off-by: Tingting_Wan <[email protected]> * Update kedro-datasets/kedro_datasets/spark/spark_stream_dataset.py Co-authored-by: Nok Lam Chan <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * rename the dataset Signed-off-by: Tingting_Wan <[email protected]> * resolve comments Signed-off-by: Tingting_Wan <[email protected]> * fix format and pylint Signed-off-by: Tingting_Wan <[email protected]> * Update kedro-datasets/kedro_datasets/spark/README.md Co-authored-by: Deepyaman Datta <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> * add unit tests and SparkStreamingDataset in init.py Signed-off-by: Tingting_Wan <[email protected]> * add unit tests Signed-off-by: Tingting_Wan <[email protected]> * update test_save Signed-off-by: Tingting_Wan <[email protected]> * formatting Signed-off-by: Tingting_Wan <[email protected]> * formatting Signed-off-by: Tingting_Wan <[email protected]> * formatting Signed-off-by: Tingting_Wan <[email protected]> * formatting Signed-off-by: Tingting_Wan <[email protected]> * lint Signed-off-by: Tingting_Wan <[email protected]> * lint Signed-off-by: Tingting_Wan <[email protected]> * lint Signed-off-by: Tingting_Wan <[email protected]> * update test cases Signed-off-by: Tingting_Wan <[email protected]> * add negative test Signed-off-by: Tingting_Wan <[email protected]> * remove code snippets fpr testing Signed-off-by: Tingting_Wan <[email protected]> * lint Signed-off-by: Tingting_Wan <[email protected]> * update tests Signed-off-by: Tingting_Wan <[email protected]> * update test and remove redundacy Signed-off-by: Tingting_Wan <[email protected]> * linting Signed-off-by: Tingting_Wan <[email protected]> * refactor file format Signed-off-by: Tom Kurian <[email protected]> * fix read me file Signed-off-by: Tom Kurian <[email protected]> * docs: Add community contributions (#199) * Add community contributions Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Use newer link to docs Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> --------- Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * adding test for raise error Signed-off-by: Tingting_Wan <[email protected]> * update test and remove redundacy Signed-off-by: Tingting_Wan <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * linting Signed-off-by: Tingting_Wan <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * refactor file format Signed-off-by: Tom Kurian <[email protected]> * fix read me file Signed-off-by: Tom Kurian <[email protected]> * adding test for raise error Signed-off-by: Tingting_Wan <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * fix readme file Signed-off-by: Tom Kurian <[email protected]> * fix readme Signed-off-by: Tom Kurian <[email protected]> * fix conflicts Signed-off-by: Tom Kurian <[email protected]> * fix ci erors Signed-off-by: Tom Kurian <[email protected]> * fix lint issue Signed-off-by: Tom Kurian <[email protected]> * update class documentation Signed-off-by: Tom Kurian <[email protected]> * add additional test cases Signed-off-by: Tom Kurian <[email protected]> * add s3 read test cases Signed-off-by: Tom Kurian <[email protected]> * add s3 read test cases Signed-off-by: Tom Kurian <[email protected]> * add s3 read test case Signed-off-by: Tom Kurian <[email protected]> * test s3 read Signed-off-by: Tom Kurian <[email protected]> * remove redundant test cases Signed-off-by: Tom Kurian <[email protected]> * fix streaming dataset configurations Signed-off-by: Tom Kurian <[email protected]> * update streaming datasets doc Signed-off-by: Tingting_Wan <[email protected]> * resolve comments re documentation Signed-off-by: Tingting_Wan <[email protected]> * bugfix lint Signed-off-by: Tingting_Wan <[email protected]> * update link Signed-off-by: Tingting_Wan <[email protected]> * revert the changes on CI Signed-off-by: Nok Chan <[email protected]> * test(docker): remove outdated logging-related step (#207) * fixkedro- docker e2e test Signed-off-by: Nok Chan <[email protected]> * fix: add timeout to request to satisfy bandit lint --------- Signed-off-by: Nok Chan <[email protected]> Co-authored-by: Deepyaman Datta <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * ci: ensure plugin requirements get installed in CI (#208) * ci: install the plugin alongside test requirements * ci: install the plugin alongside test requirements * Update kedro-airflow.yml * Update kedro-datasets.yml * Update kedro-docker.yml * Update kedro-telemetry.yml * Update kedro-airflow.yml * Update kedro-datasets.yml * Update kedro-airflow.yml * Update kedro-docker.yml * Update kedro-telemetry.yml * ci(telemetry): update isort config to correct sort * Don't use profile ¯\_(ツ)_/¯ Signed-off-by: Deepyaman Datta <[email protected]> * chore(datasets): remove empty `tool.black` section * chore(docker): remove empty `tool.black` section --------- Signed-off-by: Deepyaman Datta <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * ci: Migrate the release workflow from CircleCI to GitHub Actions (#203) * Create check-release.yml * change from test pypi to pypi * split into jobs and move version logic into script * update github actions output * lint * changes based on review * changes based on review * fix script to not append continuously * change pypi api token logic Signed-off-by: Tom Kurian <[email protected]> * build: Relax Kedro bound for `kedro-datasets` (#140) * Less strict pin on Kedro for datasets Signed-off-by: Merel Theisen <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * ci: don't run checks on both `push`/`pull_request` (#192) * ci: don't run checks on both `push`/`pull_request` * ci: don't run checks on both `push`/`pull_request` * ci: don't run checks on both `push`/`pull_request` * ci: don't run checks on both `push`/`pull_request` Signed-off-by: Tom Kurian <[email protected]> * chore: delete extra space ending check-release.yml (#210) Signed-off-by: Tom Kurian <[email protected]> * ci: Create merge-gatekeeper.yml to make sure PR only merged when all tests checked. (#215) * Create merge-gatekeeper.yml * Update .github/workflows/merge-gatekeeper.yml --------- Co-authored-by: Sajid Alam <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * ci: Remove the CircleCI setup (#209) * remove circleci setup files and utils * remove circleci configs in kedro-telemetry * remove redundant .github in kedro-telemetry * Delete continue_config.yml * Update check-release.yml * lint * increase timeout to 40 mins for docker e2e tests Signed-off-by: Tom Kurian <[email protected]> * feat: Dataset API add `save` method (#180) * [FEAT] add save method to APIDataset Signed-off-by: jmcdonnell <[email protected]> * [ENH] create save_args parameter for api_dataset Signed-off-by: jmcdonnell <[email protected]> * [ENH] add tests for socket + http errors Signed-off-by: <[email protected]> Signed-off-by: jmcdonnell <[email protected]> * [ENH] check save data is json Signed-off-by: <[email protected]> Signed-off-by: jmcdonnell <[email protected]> * [FIX] clean code Signed-off-by: jmcdonnell <[email protected]> * [ENH] handle different data types Signed-off-by: jmcdonnell <[email protected]> * [FIX] test coverage for exceptions Signed-off-by: jmcdonnell <[email protected]> * [ENH] add examples in APIDataSet docstring Signed-off-by: jmcdonnell <[email protected]> * sync APIDataSet from kedro's `develop` (#184) * Update APIDataSet Signed-off-by: Nok Chan <[email protected]> * Sync ParquetDataSet Signed-off-by: Nok Chan <[email protected]> * Sync Test Signed-off-by: Nok Chan <[email protected]> * Linting Signed-off-by: Nok Chan <[email protected]> * Revert Unnecessary ParquetDataSet Changes Signed-off-by: Nok Chan <[email protected]> * Sync release notes Signed-off-by: Nok Chan <[email protected]> --------- Signed-off-by: Nok Chan <[email protected]> Signed-off-by: jmcdonnell <[email protected]> * [FIX] remove support for delete method Signed-off-by: jmcdonnell <[email protected]> * [FIX] lint files Signed-off-by: jmcdonnell <[email protected]> * [FIX] fix conflicts Signed-off-by: jmcdonnell <[email protected]> * [FIX] remove fail save test Signed-off-by: jmcdonnell <[email protected]> * [ENH] review suggestions Signed-off-by: jmcdonnell <[email protected]> * [ENH] fix tests Signed-off-by: jmcdonnell <[email protected]> * [FIX] reorder arguments Signed-off-by: jmcdonnell <[email protected]> --------- Signed-off-by: jmcdonnell <[email protected]> Signed-off-by: <[email protected]> Signed-off-by: Nok Chan <[email protected]> Co-authored-by: jmcdonnell <[email protected]> Co-authored-by: Nok Lam Chan <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * ci: Automatically extract release notes for GitHub Releases (#212) * ci: Automatically extract release notes Signed-off-by: Ankita Katiyar <[email protected]> * fix lint Signed-off-by: Ankita Katiyar <[email protected]> * Raise exceptions Signed-off-by: Ankita Katiyar <[email protected]> * Lint Signed-off-by: Ankita Katiyar <[email protected]> * Lint Signed-off-by: Ankita Katiyar <[email protected]> --------- Signed-off-by: Ankita Katiyar <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * feat: Add metadata attribute to datasets (#189) * Add metadata attribute to all datasets Signed-off-by: Ahdra Merali <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * feat: Add ManagedTableDataset for managed Delta Lake tables in Databricks (#206) * committing first version of UnityTableCatalog with unit tests. This datasets allows users to interface with Unity catalog tables in Databricks to both read and write. Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * renaming dataset Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * adding mlflow connectors Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * fixing mlflow imports Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * cleaned up mlflow for initial release Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * cleaned up mlflow references from setup.py for initial release Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * fixed deps in setup.py Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * adding comments before intiial PR Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * moved validation to dataclass Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * bug fix in type of partition column and cleanup Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * updated docstring for ManagedTableDataSet Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * added backticks to catalog Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * fixing regex to allow hyphens Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/test_requirements.txt Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Jannic <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * adding backticks to catalog Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> * Require pandas < 2.0 for compatibility with spark < 3.4 Signed-off-by: Jannic Holzer <[email protected]> * Replace use of walrus operator Signed-off-by: Jannic Holzer <[email protected]> * Add test coverage for validation methods Signed-off-by: Jannic Holzer <[email protected]> * Remove unused versioning functions Signed-off-by: Jannic Holzer <[email protected]> * Fix exception catching for invalid schema, add test for invalid schema Signed-off-by: Jannic Holzer <[email protected]> * Add pylint ignore Signed-off-by: Jannic Holzer <[email protected]> * Add tests/databricks to ignore for no-spark tests Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Nok Lam Chan <[email protected]> * Update kedro-datasets/kedro_datasets/databricks/managed_table_dataset.py Co-authored-by: Nok Lam Chan <[email protected]> * Remove spurious mlflow test dependency Signed-off-by: Jannic Holzer <[email protected]> * Add explicit check for database existence Signed-off-by: Jannic Holzer <[email protected]> * Remove character limit for table names Signed-off-by: Jannic Holzer <[email protected]> * Refactor validation steps in ManagedTable Signed-off-by: Jannic Holzer <[email protected]> * Remove spurious checks for table and schema name existence Signed-off-by: Jannic Holzer <[email protected]> --------- Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> Co-authored-by: Danny Farah <[email protected]> Co-authored-by: Danny Farah <[email protected]> Co-authored-by: Nok Lam Chan <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * docs: Update APIDataset docs and refactor (#217) * Update APIDataset docs and refactor * Acknowledge community contributor * Fix more broken doc Signed-off-by: Nok Chan <[email protected]> * Lint Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Fix release notes of upcoming kedro-datasets --------- Signed-off-by: Nok Chan <[email protected]> Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> Co-authored-by: Juan Luis Cano Rodríguez <[email protected]> Co-authored-by: Jannic <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * feat: Release `kedro-datasets` version `1.3.0` (#219) * Modify release version and RELEASE.md Signed-off-by: Jannic Holzer <[email protected]> * Add proper name for ManagedTableDataSet Signed-off-by: Jannic Holzer <[email protected]> * Update kedro-datasets/RELEASE.md Co-authored-by: Juan Luis Cano Rodríguez <[email protected]> * Revert lost semicolon for release 1.2.0 Signed-off-by: Jannic Holzer <[email protected]> --------- Signed-off-by: Jannic Holzer <[email protected]> Co-authored-by: Juan Luis Cano Rodríguez <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * docs: Fix APIDataSet docstring (#220) * Fix APIDataSet docstring Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Add release notes Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> * Separate [docs] extras from [all] in kedro-datasets Fix gh-143. Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> --------- Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * Update kedro-datasets/tests/spark/test_spark_streaming_dataset.py Co-authored-by: Deepyaman Datta <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * Update kedro-datasets/kedro_datasets/spark/spark_streaming_dataset.py Co-authored-by: Deepyaman Datta <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * Update kedro-datasets/setup.py Co-authored-by: Deepyaman Datta <[email protected]> Signed-off-by: Tom Kurian <[email protected]> * fix linting issue Signed-off-by: Tom Kurian <[email protected]> --------- Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> Signed-off-by: Tingting_Wan <[email protected]> Signed-off-by: Juan Luis Cano Rodríguez <[email protected]> Signed-off-by: Ankita Katiyar <[email protected]> Signed-off-by: Nok Chan <[email protected]> Signed-off-by: Tom Kurian <[email protected]> Signed-off-by: Deepyaman Datta <[email protected]> Signed-off-by: Merel Theisen <[email protected]> Signed-off-by: jmcdonnell <[email protected]> Signed-off-by: <[email protected]> Signed-off-by: Ahdra Merali <[email protected]> Signed-off-by: Danny Farah <[email protected]> Signed-off-by: Jannic Holzer <[email protected]> Co-authored-by: Juan Luis Cano Rodríguez <[email protected]> Co-authored-by: Tingting Wan <[email protected]> Co-authored-by: Nok Lam Chan <[email protected]> Co-authored-by: Deepyaman Datta <[email protected]> Co-authored-by: Nok Lam Chan <[email protected]> Co-authored-by: Ankita Katiyar <[email protected]> Co-authored-by: Juan Luis Cano Rodríguez <[email protected]> Co-authored-by: Tom Kurian <[email protected]> Co-authored-by: Sajid Alam <[email protected]> Co-authored-by: Merel Theisen <[email protected]> Co-authored-by: McDonnellJoseph <[email protected]> Co-authored-by: jmcdonnell <[email protected]> Co-authored-by: Ahdra Merali <[email protected]> Co-authored-by: Jannic <[email protected]> Co-authored-by: Danny Farah <[email protected]> Co-authored-by: Danny Farah <[email protected]> Co-authored-by: kuriantom369 <[email protected]>
Description
Resolves kedro-org/kedro#2153
Development notes
The migration from CircleCI to Github Actions will be have to done in parts. This PR addresses the following parts -
kedro-docker
,kedro-datasets
,kedro-telemetry
&kedro-airflow
kedro-docker
,kedro-datasets
,kedro-telemetry
&kedro-airflow
kedro-docker
,kedro-telemetry
&kedro-airflow
unit-test
job to work with both linux and windowsTO DOs
The following will be added in separate PRs
Unit tests on windows forkedro-docker
,kedro-datasets
,kedro-telemetry
&kedro-airflow
End-to-end tests forkedro-docker
,kedro-datasets
,kedro-telemetry
&kedro-airflow
Notes
kedro-datasets
) because of this line inkedro-datasets/test_requirements.txt
kedro-datasets/tests/tensorflow/test_tensorflow_model_dataset.py
totrufflehog-ignore.txt
because secret-scan was complaining about this partREADME.md
files of all plugins to trigger these tests and will revert them before merging.macos-latest
tomatrix.os
list in theunit-tests
job. Should we do this? The Github action workflows are slow mostly forkedro-datasets
(at least the way it's set up now) but they run in parallel.Checklist
RELEASE.md
file