Skip to content

Make suggested resume commands copy-pasteable even when they extend over multiple lines#4709

Draft
david-stanley-94 wants to merge 4 commits intokedro-org:mainfrom
david-stanley-94:feat/copy-pasteable-log-outputs
Draft

Make suggested resume commands copy-pasteable even when they extend over multiple lines#4709
david-stanley-94 wants to merge 4 commits intokedro-org:mainfrom
david-stanley-94:feat/copy-pasteable-log-outputs

Conversation

@david-stanley-94
Copy link

@david-stanley-94 david-stanley-94 commented May 8, 2025

Description

  • Adds a plain text log handler for log outputs that users benefit from being copy pasteable, such as suggested resume commands after pipeline failure
  • Ensures such outputs are copy pasteable even for medium and large pipelines whose resume commands would span multiple lines
  • The existing rich handler formats these with line breaks for text wrapping, padding spaces, etc, which means commands are not directly copy pasteable

Development notes

  • Add a plain text log handler in logging.py
  • Configure the plain text handler in framework/project/rich_logging.yml (where the rich handler is configured)
  • Add the plain text handler as a property to the kedro runner in runner/runner.py (where the rich handler is used)
  • Use the plain text handler for outputting the postfix variable in the _suggest_resume_scenario method, in runner/runner.py. The text preceding the suggested command is still to use the rich handler.

On testing, gives the following output for arbitrary error raised in a node:

image

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

@david-stanley-94 david-stanley-94 requested a review from merelcht as a code owner May 8, 2025 13:50
@david-stanley-94 david-stanley-94 marked this pull request as draft May 8, 2025 13:50
Signed-off-by: David Stanley <david_stanley@mckinsey.com>
Signed-off-by: David Stanley <david_stanley@mckinsey.com>
Signed-off-by: David Stanley <david_stanley@mckinsey.com>
…ver multiple lines

By logging without formatting (e.g. line breaks for text wrap, padding, etc)

Signed-off-by: David Stanley <david_stanley@mckinsey.com>
@david-stanley-94 david-stanley-94 force-pushed the feat/copy-pasteable-log-outputs branch from a98b2f3 to eb2d69d Compare May 8, 2025 16:03
@astrojuanlu
Copy link
Contributor

Thanks for this PR @david-stanley-94 !

A couple of questions:

  1. I see you're focusing specifically on resume suggestions. Are there other situations that make the logs non copy-pasteable we should look at?
  2. Is it feasible to achieve this without even touching the Kedro core? For example, by configuring a plain logger coming from a plugin or from a custom module. Would it work?

@david-stanley-94
Copy link
Author

Thanks for this PR @david-stanley-94 !

A couple of questions:

  1. I see you're focusing specifically on resume suggestions. Are there other situations that make the logs non copy-pasteable we should look at?
  2. Is it feasible to achieve this without even touching the Kedro core? For example, by configuring a plain logger coming from a plugin or from a custom module. Would it work?
  1. Have searched the kedro repo for _logger, logger., and can only find one other instance, in the __init__.py file of the ipython dir, where the plain text handler might be useful:
def _resolve_project_path(
    path: str | None = None, local_namespace: dict[str, Any] | None = None
) -> Path:
    """
    Resolve the project path to use with reload_kedro, updating or adding it
    (in-place) to the local ipython Namespace (``local_namespace``) if necessary.

    Arguments:
        path: the path to use as a string object
        local_namespace: Namespace with local variables of the scope where the line
            magic is invoked in a dict.
    """
    if path:
        project_path = Path(path).expanduser().resolve()
    else:
        if (
            local_namespace
            and local_namespace.get("context")
            and hasattr(local_namespace["context"], "project_path")
        ):
            project_path = local_namespace["context"].project_path
        else:
            project_path = _find_kedro_project(Path.cwd())
        if project_path:
            logger.info(
                "Resolved project path as: %s.\nTo set a different path, run "
                "'%%reload_kedro <project_root>'",
                project_path,
            )
  1. Good question! Let me try to look over the kedro plugins docs, etc, to better understand how that might work. Any additional guidance appreciated.

@merelcht
Copy link
Member

Is this a lot different from the suggestion in the docs https://docs.kedro.org/en/stable/logging/index.html#how-to-use-plain-console-logging ?

@david-stanley-94
Copy link
Author

david-stanley-94 commented May 13, 2025

Is this a lot different from the suggestion in the docs https://docs.kedro.org/en/stable/logging/index.html#how-to-use-plain-console-logging ?

That's a work around of sorts (for anyone working with medium to large sized pipelines), but I see the issue itself as being that kedro gives suggested console commands implicitly for users to copy paste, that are not directly copy pasteable when formatted using rich.

One alternative is to move away from rich formatting, to another formatter that has more compatibility with copy pasting, but I understand from previous comments that is no easy task due to an indirect dependency on rich. So another alternative is to selectively change the formatting of elements intended to be copy pasteable, to use a plain text handler.

@noklam
Copy link
Contributor

noklam commented May 14, 2025

I understand the problem here when rich breaks useful information into chunks and make it unsable. I wonder if there are way to solve this in a generic way. This PR target the resume suggestion, but the line break are problematic for things like file path where you can normally do Cmd + Click.

I see there are option for soft-wrapping but it doesn't seem to be available for rich.pretty.install.
https://rich.readthedocs.io/en/stable/console.html

@merelcht
Copy link
Member

After running both the console logger and your plain logger, I see the main difference is that with your logger you don't get stuff like 2025-05-14 16:36:35,637 - kedro.runner.sequential_runner - WARNING in front of the log lines.

I feel like this is kind of a personal preference thing and I'm not sure this actually adds significant value that merits this being added to the Kedro core. With the console logger you can already achieve the goal of copy-pasteable log lines.

@david-stanley-94
Copy link
Author

Yeah the aim is to solve issues akin to that described in #3276 , as opposed to more generally changing how logging is done. Ideally the logging formatting/behaviour would stay exactly as it is, just that long resume commands are copy pasteable (as implicitly intended and is the case for short ones) and long filepaths are correctly clickable (as is the case for short ones).

One use case would be running a large pipeline taking 30 mins to run, that breaks almost at the end. The user can copy paste out the resume command, and then fix the bug. Currently they will then have to spend some time (several minutes? I forget how long this has taken me before) fixing the broken command, or else rerun from the beginning (taking 30 mins). Ideally, they would just be able to copy paste straight back into the terminal without delay (a couple of seconds).

@astrojuanlu
Copy link
Contributor

I haven't said anything here because I haven't had time to properly review the changes but I think we should not underestimate the impact of Kedro logs not being easily copy-pasteable by default. It's something that bites me almost every week.

@ravi-kumar-pilla
Copy link
Contributor

Hi @david-stanley-94 ,

Thank you for the contribution. Apologies for the delayed response on this.

Have you tried Rich Text and would it be helpful if we use it in areas where copy-pasting should be supported, instead of adding a new handler ?

I tried using rich Text for the example screenshot and it outputs similar to what you have with the new handler -

image

Let me know what you think of this. Thank you

@david-stanley-94
Copy link
Author

david-stanley-94 commented Jan 12, 2026

Thanks for the suggestion, @ravi-kumar-pilla! Would need to test if that works as an alternative.

I wanna say I tested this at the time and that it didn't, but tbh it's so long ago now I don't remember, and it seems I didn't document what things, other than a different logger, that I had tried (sorry about that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

6 participants