Make it possible to use Kedro after uninstalling Rich.

[lrcouto]

Description

Allows Kedro to work regardless of the rich library being installed. At this moment, installing Kedro will still install rich, but the user will have the option to remove the library and still have everything work.

Because cookiecutter versions 2.3 and above depend on rich, it is also necessary to remove the version installed automatically by Kedro and install a compatible one to be able to run without rich.

Logging, prompts, and the kedro ipython command will work without rich given a compatible cookiecutter version is also being used. Our own unit tests will not, they will fail without rich.

Development notes

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

[SajidAlamQB]

Is the basic idea that users who don't install rich should default to this? So for users who already have rich installed and no longer want it they need to pip uninstall rich? If that is the case we should document the steps they need to take.

[SajidAlamQB]

@noklam do you have any thoughts on this?

[noklam]

My thought on this it shouldn't touch the log handler.

If the custom logger has Rich handler, it should raise error when it is not installed. To get rid of this the solution is just remove rich from the handler.

The thing that user cannot change now is all the traceback error / CLI that ties with rich. Essentially one cannot run Kedro without rich. @astrojuanlu

I am surprised the _ProjectLogging isn't changed here.

[lrcouto]

@SajidAlamQB yes, the way this is set up at the moment, if the user does not want rich to be used, it would need to be uninstalled.

[SajidAlamQB]

Hey @lrcouto, this is an interesting approach, I think we will also need to modify kedro/framework/project/default_logging.yml, kedro/templates/project/{{ cookiecutter.repo_name }}/conf/logging.yml as they both also point to rich.

[astrojuanlu]

The thing that user cannot change now is all the traceback error / CLI that ties with rich. Essentially one cannot run Kedro without rich.

Don't quite understand that part, could you elaborate @noklam ?

[noklam]

@astrojuanlu Alright I was confused because I see no changes on these:

kedro/kedro/logging.py

Line 35 in 9da8b37

rich.pretty.install()

and

kedro/kedro/logging.py

Lines 56 to 60 in 9da8b37

	if self.rich_tracebacks and not _is_databricks():
	# Rich traceback handling does not work on databricks. Hopefully this will be
	# fixed on their side at some point, but until then we disable it.
	# See https://github.com/Textualize/rich/issues/2455
	rich.traceback.install(**traceback_install_kwargs) # type: ignore[arg-type]

Now I realise the logic is in the early return.

My question is, why do we want to be clever about the RichHandler? Alternatively, we can have another "default" setting, and if rich is not importable we simply use another default. One thing that doesn't work is if conf/logging.yml still have rich enabled.

In that case, I think it's reasonable to fail rather than being smart about it.

kedro/kedro/framework/project/__init__.py

Lines 220 to 224 in 9da8b37

	path = os.environ.get(
	"KEDRO_LOGGING_CONFIG", Path(__file__).parent / "default_logging.yml"
	)
	logging_config = Path(path).read_text(encoding="utf-8")
	self.configure(yaml.safe_load(logging_config))

[noklam]

why do we need both handlers? would it duplicates the log?

[lrcouto]

@astrojuanlu Alright I was confused because I see no changes on these:

kedro/kedro/logging.py

Line 35 in 9da8b37

rich.pretty.install()

and

kedro/kedro/logging.py

Lines 56 to 60 in 9da8b37

	if self.rich_tracebacks and not _is_databricks():
	# Rich traceback handling does not work on databricks. Hopefully this will be
	# fixed on their side at some point, but until then we disable it.
	# See https://github.com/Textualize/rich/issues/2455
	rich.traceback.install(**traceback_install_kwargs) # type: ignore[arg-type]

Now I realise the logic is in the early return.

My question is, why do we want to be clever about the RichHandler? Alternatively, we can have another "default" setting, and if rich is not importable we simply use another default. One thing that doesn't work is if conf/logging.yml still have rich enabled.

In that case, I think it's reasonable to fail rather than being smart about it.

kedro/kedro/framework/project/__init__.py

Lines 220 to 224 in 9da8b37

	path = os.environ.get(
	"KEDRO_LOGGING_CONFIG", Path(__file__).parent / "default_logging.yml"
	)
	logging_config = Path(path).read_text(encoding="utf-8")
	self.configure(yaml.safe_load(logging_config))

I was looking into this idea of adding a non-rich default yesterday, but that would mean having to try to import rich on init.py to check if it is installed, and I was trying to avoid adding more imports. I think it's a valid idea regardless. We could have a non-rich version that gets defaulted to if rich is not present, and that would allow us to not have to mess with the logging handler. What do you think?

[astrojuanlu]

Ugh, thanks for the investigation @lrcouto. We'll give this some thought.

[merelcht]

I wouldn't pin cookiecutter so strictly, but instead clarify in docs that if users don't want to use rich they might have to downgrade their cookiecutter version.

[astrojuanlu]

In line with #2928 (comment), maybe let's rename the PR to make it clear that we're not marking rich as an optional dependency yet (and therefore this doesn't yet fully address #2928)

[merelcht]

Some minor comments, but nothing blocking.

Great work! 👍

[merelcht]

I think this section will fit better in the logging docs: https://docs.kedro.org/en/stable/logging/index.html

[lrcouto]

I thought about putting it in the logging page, as it already mentions rich a lot, but since it affects features other than logging I thought it'd fit better as more general configuration information. What do you think about adding it to both pages?

[merelcht]

Maybe name this RICH_INSTALLED?

[SajidAlamQB]

Thank you @lrcouto for taking this on, tested this manually and this looks good to me. 🌟