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

Fix messaging level when package compatibility is not satisfied #1904

Merged
merged 10 commits into from
May 15, 2024

Conversation

ravi-kumar-pilla
Copy link
Contributor

@ravi-kumar-pilla ravi-kumar-pilla commented May 15, 2024

Description

Resolves #1877

Development notes

  • Update log level to warning
  • Modify PACKAGE_REQUIREMENTS to have warning_message and min_compatible_version dict
  • Update tests

NOTE: Lint will be fixed with the PR - #1903

QA notes

  • Start kedro viz run on a kedro project which does not use kedro-datasets
  • Users should see a warning message instead of error
  • All tests should pass

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added new entries to the RELEASE.md file
  • Added tests to cover my changes

@ravi-kumar-pilla ravi-kumar-pilla marked this pull request as ready for review May 15, 2024 02:58
@ravi-kumar-pilla ravi-kumar-pilla requested review from SajidAlamQB and removed request for astrojuanlu May 15, 2024 02:59
@ravi-kumar-pilla
Copy link
Contributor Author

Hi @ankatiyar ,

Not related to this ticket but I was curious about the workflow - .github/workflows/docs-only-checks.yml

  1. I see the workflow runs the linter but the linter has commands which are related to python and some secret scan etc. How is this helpful for doc changes ?
  2. The trigger condition has .md files, so this will be triggered for all the PRs to main which have a release note and we also run backend linters separately which do almost the same.
  3. Can we run this for only 1 python version, in case this is needed for any reason ?

cc: @Huongg

Thank you

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.

Looks good, thank you @ravi-kumar-pilla!

Signed-off-by: ravi-kumar-pilla <[email protected]>
@ravi-kumar-pilla ravi-kumar-pilla merged commit a8f57ad into main May 15, 2024
37 checks passed
@ravi-kumar-pilla ravi-kumar-pilla deleted the fix/package-comp-messaging branch May 15, 2024 18:59
This was referenced May 21, 2024
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.

Fix messaging level when package compatibility is not satisfied
3 participants