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

Add deprecation notice to archived starters #3173

Merged
merged 5 commits into from
Oct 17, 2023

Conversation

ankatiyar
Copy link
Contributor

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

Close #3114

Development notes

  • Show deprecation notice if the selected starter is scheduled for archiving
  • Show deprecation notice when kedro starter list is used
  • Add a message about the upcoming add-ons to kedro new command.

Developer Certificate of Origin

We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a Signed-off-by line in the commit message. See our wiki for guidance.

If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.

Checklist

  • Read the contributing guidelines
  • Signed off each commit with a Developer Certificate of Origin (DCO)
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes
  • Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Copy link
Contributor

@SajidAlamQB SajidAlamQB left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @ankatiyar! 👍

@datajoely
Copy link
Contributor

Sorry I missed this - what is the replacement for the pyspark starter? This affects most of the QB Verticals

@SajidAlamQB
Copy link
Contributor

SajidAlamQB commented Oct 13, 2023

Sorry I missed this - what is the replacement for the pyspark starter? This affects most of the QB Verticals

I believe it will be replaced by, https://github.com/kedro-org/kedro-starters/tree/main/spaceflights-pyspark, it was introduced by @merelcht with a new suite of starters that use spaceflights as a base.

@merelcht
Copy link
Member

It would also be useful to add the deprecation mention to kedro starter list so the result is something like:

astro-airflow-iris:
  template_path: git+https://github.com/kedro-org/kedro-starters.git
  directory: astro-airflow-iris
astro-iris:
  template_path: git+https://github.com/kedro-org/kedro-starters.git
  directory: astro-airflow-iris
databricks-iris:
  template_path: git+https://github.com/kedro-org/kedro-starters.git
  directory: databricks-iris
pandas-iris (deprecated):
  template_path: git+https://github.com/kedro-org/kedro-starters.git
  directory: pandas-iris
pyspark (deprecated):
  template_path: git+https://github.com/kedro-org/kedro-starters.git
  directory: pyspark
pyspark-iris (deprecated):
  template_path: git+https://github.com/kedro-org/kedro-starters.git
  directory: pyspark-iris
spaceflights:
  template_path: git+https://github.com/kedro-org/kedro-starters.git
  directory: spaceflights
standalone-datacatalog (deprecated):
  template_path: git+https://github.com/kedro-org/kedro-starters.git
  directory: standalone-datacatalog

@datajoely
Copy link
Contributor

So I want to strongly argue we shouldn't delete the empty pyspark starter- the verticals need any empty template to build off

Copy link
Contributor

@datajoely datajoely left a comment

Choose a reason for hiding this comment

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

This would be a breaking change for many QB Vertical workflows

@ankatiyar
Copy link
Contributor Author

@datajoely I was under the impression that you can create an empty pyspark template with the add-ons workflow, I think @SajidAlamQB is working on it?

@datajoely
Copy link
Contributor

This would break the existing Alloy workflow - can you follow all the add on steps from a single command? It won't work if it's an interactive pattern.

@ankatiyar
Copy link
Contributor Author

ankatiyar commented Oct 13, 2023

This would break the existing Alloy workflow - can you follow all the add on steps from a single command? It won't work if it's an interactive pattern.

@datajoely Yeah, I believe it'll be kedro new --addons=pyspark or something like that but @SajidAlamQB @AhdraMeraliQB can comment more on it.

@datajoely
Copy link
Contributor

Either way it would make things a lot easier to keep this existing cookie cutter version I think

@datajoely
Copy link
Contributor

The main risk is that existing users refuse to go to 0.19.x because of this

@SajidAlamQB
Copy link
Contributor

@datajoely I was under the impression that you can create an empty pyspark template with the add-ons workflow, I think @SajidAlamQB is working on it?

Yes, the new add-ons flow will have an option to create an empty pyspark template, and yes I believe @lrcouto is working on #2873 which will allow you to use add-ons from a single command.

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@datajoely
Copy link
Contributor

I would strongly request that we keep both the pyspark and blank Kedro starters in cookiecutter format as is

Copy link
Contributor

@stichbury stichbury left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@merelcht
Copy link
Member

Discussed with the team that the deprecation warnings will still go in, because they're blocking the 0.18.14 release. I will connect with @datajoely offline to discuss the best approach of serving Alloy/verticals needs with the new project creation flow and starter structure.

@ankatiyar ankatiyar enabled auto-merge (squash) October 16, 2023 14:18
@ankatiyar ankatiyar merged commit 1cde72c into main Oct 17, 2023
63 of 72 checks passed
@ankatiyar ankatiyar deleted the deprecation-warning-starters branch October 17, 2023 09:16
adamkells pushed a commit to adamkells/kedro that referenced this pull request Oct 30, 2023
* Add deprecation notice to archived starters

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

* Add '(deprecated)' to the names of the starters

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

---------

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Adam Kells <adamjkells93@gmail.com>
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

Successfully merging this pull request may close these issues.

Add a deprecation notice to signal the archiving of starters + new project creation flow
5 participants