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 log folds/groups/sections for CI services #2967

Closed
wants to merge 7 commits into from

Conversation

cognifloyd
Copy link
Contributor

@cognifloyd cognifloyd commented Nov 16, 2020

Add output when molecule is running in some CI services (Travis, Github Actions, GitLab Pipelines) to make the logs easier to jump through.

This is a follow up to #2851

PR Type

  • Feature Pull Request

CI Docs:

  • Travis log folding is explained here, here, and here (CI vars documented here).
  • Github Action output groups are explained here (CI vars documented here).
  • GitLab Pipelines collapsible sections are explained here (CI vars documented here).

Screenshots and Tests

Travis CI:
  • implement based on Travis CI experience as documented in various places.
  • test/validate molecule with Travis CI. Tested with this job on Travis (nb molecule works just fine in this job, but the job itself failed because the tests haven't been fully ported to molecule 3).
  • take a screenshot with Travis CI.
    TravisCI Screenshot
Github Actions:
  • implement based on Github Actions docs.
  • test/validate molecule with Github Actions. Tested with this job on Github Actions.
  • take a screenshot with Github Actions.
    Github Actions Screenshot
GitLab Pipelines:
  • implement based on GitLab Pipelines docs.
  • test/validate molecule with GitLab Pipelines. Tested with this job on GitLab Pipelines
  • take a screenshot with GitLab Pipelines.
    GitLabs Pipelines Screenshot
    nb: The title in GitLab has to be all one color, or it ends up with funky output.

@cognifloyd cognifloyd force-pushed the travis-folding branch 4 times, most recently from 73b27f4 to 5fddbb4 Compare November 16, 2020 02:15
@cognifloyd cognifloyd force-pushed the travis-folding branch 13 times, most recently from b6f9eb8 to 2e52642 Compare November 16, 2020 20:29
@cognifloyd
Copy link
Contributor Author

Tested on all 3 platforms. I think this is ready to merge.

@cognifloyd
Copy link
Contributor Author

Just noticed a straggler TODO (already done) so I rewrote history and removed it.

@ssbarnea
Copy link
Member

ssbarnea commented Nov 17, 2020

@cognifloyd I cannot state how happy I am to see this implemented as I wanted to see it myself for years. Sadly my iterm does not yet support this feature and very few terminals seems to support it.

i am not surprised it became popular on CI systems as doing it in HTML is quite easy.

I will be happy to list this as no1 feature of 3.2 release. Still, I would like to request few changes to prepare it for the future: mainly name the feature collapsable_sections, foldable_sections or similar and avoid naming it and the classes around "CI".

While the current implementation works only with few particular CI systems, it is not something unique to CI and sooner or later will be enabled for terminals too. Make it generic and we enable it when we detect a context that supports it, and do nothing when is not supported.

BTW. Can you please add a comment here with expected format used by all supported systems? I looked at the code and I am not very pleased around the extra conditionals used to render these. I wonder if we can make use of rich features to implement this in a less intrusive way.

In fact I am current working on https://github.com/pycontribs/enrich which will be used by molecule and maybe we can make the folding a native support of the Console class, so the application would not need to add any logic specific to particular CI systems. The application would only need to mention that a message starts a section or ends it, rending being taking case at a different level.

@ssbarnea ssbarnea added this to the 3.2.0 milestone Nov 17, 2020
@cognifloyd
Copy link
Contributor Author

cognifloyd commented Nov 17, 2020

Following the chain of links:
https://gitlab.freedesktop.org/terminal-wg/specifications/-/issues/3 is a discussion around adding folding to terminal emulator spec (seems unlikely to be accepted based on the primarily negative response)
https://gitlab.freedesktop.org/terminal-wg/specifications/-/issues/4 takes a step back from the presentation (folding) to add semantic markers that could be used by a terminal emulator to add log folding

More description of the propsal in #4 is at http://per.bothner.com/blog/2019/shell-integration-proposal/

So there might be escape sequences at some point that can denote the start/end of output. Terminal emulators might add support for turning that into folding or collapsible sections. Support in CI environments would also need them to support that standardized sequence.

So, maybe I'll call this mark_sections. And I'll divide this into two utility functions:

  • get_section_marker_style() -> Optional[str] returns the name of the section_marker_style. For now this would be one of travis_ci_fold, github_action_group, gitlab_ci_section, or None.get_section_marker_style() -> Optional[str] returns the name of the section_marker_style
  • dispatch_section_markers() very similar to the current implementation.

Or, instead of doing a function, we can actually expand this out into a more extensible command pattern.

from abc import ABC, abstractmethod

class SectionMarker(ABC):
    @classmethod
    @abstractmethod
    def can_handle():
        """"Investigate environment to see if section markers are supported."""

    @abstractmethod
    def start(section_id: str, title: str = None):
        """"Print the start marker. title may include markers and emoji."""

    @abstractmethod
    def end(section_id: str):
        """"Print the end marker."""

in lib/molecule/command/base.py:

def subcommand_markers(func):
    section_marker = None
    for section_marker_class in get_section_marker_classes():
        if section_marker_class.can_handle():
            section_marker = section_marker_class()
            break
    if not section_marker:
        return func

    @functools.wraps(func)
    def section_marker_wrapper(config, subcommand):
        scenario = config.scenario.name
        section_marker.start(
            section_id=f"{scenario}.{subcommand}",
            title=f"Molecule {scenario} > {subcommand}",  # for section_marker handlers that handler markers internally
            title_with_markers=f"[section_title]Molecule[/] [scenario]{scenario}[/] > [action]{subcommand}[/]",
        )
        try:
            return func(config, subcommand)
        finally:
            section_marker.end(section_id=f"{scenario}.{subcommand}")

@subcommand_markers
def execute_subcommand(config, subcommand):

This is a lot heavier weight in terms of objects. Do we need to go this far?

@cognifloyd cognifloyd changed the base branch from master to devel November 17, 2020 15:53
@cognifloyd
Copy link
Contributor Author

I switched the target branch to devel - I'm guessing this is for 3.2 and master is for 3.1?
I'll work on refactoring these in a bit.

@ssbarnea ssbarnea closed this Nov 17, 2020
@ssbarnea ssbarnea deleted the branch ansible:devel November 17, 2020 16:03
@ssbarnea
Copy link
Member

ssbarnea commented Nov 17, 2020

@cognifloyd nope. the only branch receiving patches is the default branch (master). All the others are temporary and usually deleted after merge and reused later.

This explains why I was surprised seeing your PR disappearing.

BTW, As you probably seen I already merged the changes to logger. Now I am looking on how to make it easy for you to hook the section feature.

I wonder if anyone supports nested sections or if everyone made them at a single level. I am asking this because single level sectioning could make it easier to implement as we can keep status in/out section inside the logger and close the current section before starting another one, not forcing user to close it.

@ssbarnea
Copy link
Member

@cognifloyd Look at https://github.com/ansible-community/molecule/pull/2975/files which I created to facilitate your change.

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.

2 participants