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 reporter handler, output handler and reporter manager class structure to conda #13818

Open
wants to merge 15 commits into
base: feature/reporters-backends
Choose a base branch
from
Open
128 changes: 128 additions & 0 deletions conda/common/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@
# SPDX-License-Identifier: BSD-3-Clause
"""Common I/O utilities."""

from __future__ import annotations

import json
import logging
import os
import signal
import sys
from abc import ABC, abstractmethod
from collections import defaultdict
from concurrent.futures import Executor, Future, ThreadPoolExecutor, _base, as_completed
from concurrent.futures.thread import _WorkItem
Expand All @@ -20,6 +23,7 @@
from os.path import dirname, isdir, isfile, join
from threading import Event, Lock, RLock, Thread
from time import sleep, time
from typing import Dict, Union

from ..auxlib.decorators import memoizemethod
from ..auxlib.logz import NullHandler
Expand Down Expand Up @@ -689,6 +693,130 @@
os.makedirs(dirname(self.record_file))


Reporter = Dict[str, Union[bool, int, str]]
DetailRecord = Dict[str, Union[str, int, bool]]
kenodegard marked this conversation as resolved.
Show resolved Hide resolved


class ReporterHandlerBase(ABC):
"""
Base class for all reporter handlers
"""

@abstractmethod
def detail_view(self, data: DetailRecord, **kwargs) -> str:
"""
This method is responsible for generating the output in a "tabular" format.
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved
"""
...

@abstractmethod
def string_view(self, data: str, **kwargs) -> str:
"""
This method returns simple string output.
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved
"""
...


class ConsoleHandler(ReporterHandlerBase):
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved
"""
Reporter handler for standard output (classic output).
"""

def detail_view(self, data: DetailRecord, **kwargs) -> str:
table_str = ""
longest_header = len(sorted(data.keys(), key=len).pop())
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved

for header, value in data.items():
row_header = header.ljust(longest_header, " ")
table_str += f"{row_header} : {value}\n"
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved

return table_str

def string_view(self, data: str, **kwargs) -> str:
return data


class JSONHandler(ReporterHandlerBase):
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved
"""
Reporter handler for JSON output.
"""

def detail_view(self, data: DetailRecord, **kwargs) -> str:
json_str = json.dumps(data)

Check warning on line 745 in conda/common/io.py

View check run for this annotation

Codecov / codecov/patch

conda/common/io.py#L745

Added line #L745 was not covered by tests

return json_str

Check warning on line 747 in conda/common/io.py

View check run for this annotation

Codecov / codecov/patch

conda/common/io.py#L747

Added line #L747 was not covered by tests

def string_view(self, data: str, **kwargs) -> str:
json_str = json.dumps(data)

Check warning on line 750 in conda/common/io.py

View check run for this annotation

Codecov / codecov/patch

conda/common/io.py#L750

Added line #L750 was not covered by tests

return json_str

Check warning on line 752 in conda/common/io.py

View check run for this annotation

Codecov / codecov/patch

conda/common/io.py#L752

Added line #L752 was not covered by tests


class OutputHandlerBase(ABC):
"""
Base class for all output handlers
"""

@property
@abstractmethod
def name(self) -> str:
"""
Name of the output handler; this will be how this output handler is referenced
in configuration files
"""
...

@abstractmethod
def render(self, renderable: str, **kwargs) -> None:
"""Render implementation goes here"""
...


class StdoutHandler(OutputHandlerBase):
"""
Handles writing output strings to stdout.
"""

@property
def name(self) -> str:
return "stdout"

def render(self, renderable: str, **kwargs) -> None:
sys.stdout.write(renderable)


class ReporterManager:
"""
This is the glue that holds together the ``ReporterHandler`` implementations with the
``OutputHandler`` implementations. A single ``render`` method is provided for rendering
our configured reporter handlers.
"""

def __init__(self, reporters: tuple[Reporter, ...]) -> None:
self._reporters = reporters
self._reporter_handlers = {

Check warning on line 797 in conda/common/io.py

View check run for this annotation

Codecov / codecov/patch

conda/common/io.py#L796-L797

Added lines #L796 - L797 were not covered by tests
"console": ConsoleHandler(),
"json": JSONHandler(),
}
self._output_handlers = {

Check warning on line 801 in conda/common/io.py

View check run for this annotation

Codecov / codecov/patch

conda/common/io.py#L801

Added line #L801 was not covered by tests
"stdout": StdoutHandler(),
}

def render(self, name, data, **kwargs) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the usage here is intended to be:

manager = ReporterManager(...)

manager.render("string_view", "Hello world!")
manager.render("detail_view", {"foo": true})

This is unwieldy, ideally the render function should be smart enough to figure it out for us (much like how print and rich simply figure it out for us):

manager = ReporterManager(...)

manager.render("Hello world!")
manager.render({"foo": true})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, Ken!
The reason we decided to have two methods (one printing strings, one tabular data) in the first place is because a single conda command prints data in different forms depending on the flags used with it.
For example, conda info --base simply prints a string (the root prefix), whereas conda info prints tabular data (data printed in two columns; key, value pairs).
You are suggesting that reporter_manager.render(data) should itself look at the context, flags passed etc, and print the data in the appropriate format.
For example the render method will find that --base flag has been passed, so instead of printing "key: value", it will print str(value). Therefore there will need to be some if: else: logic in the render method.
But the render method in the ReporterManager class is supposed to be a general class method that can be used by any conda command.
It is possible that some other conda command used with --base flag DOES NOT print a string, but a key-value pair.
Therefore the render method should not be responsible for everything.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I do not expect the render method to know anything about the context, nor should it contain any logic to determining what is rendered.

I expect render to know that a str is a string and how to print it (using string_view). It should also know that a dict is a mapping and how to print it (using detail_view).

I expect something like the following to happen:

def conda_info(args):
    info = {...}

    if args.base:
        # conda info --base
        manager.render(info["base"])
    elif args.envs:
        # conda info --envs
        manager.render(info["envs"])
    elif ...:
        ...
    else:
        # conda info
        manager.render(info)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, Ken!
I now understand your suggested changes better. Thank you.

I expect render to know that a str is a string and how to print it (using string_view). It should also know that a dict is a mapping and how to print it (using detail_view).

The render method is intended to point to a type of "template". The methods that we’ve defined on the ReporterHandler classes are these templates. For now, we only have "string_view" and "detail_view" which depend on the data type they receive.
But not all of these "templates"/"views" will necessarily depend on the data type passed in. Therefore we do not want to create any map between which data type was passed in and which "view" method gets called.

Does that make the reasoning behind this implementation clearer? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

But not all of these "templates"/"views" will necessarily depend on the data type passed in.

If that's the case, a compelling example is necessary to sway my opinion. The implementation and the arguments for why we want it this way look and sound like we are over engineering. It's ok for us to over engineer but we need to have documented examples for why we are doing this.

In any case, I expect the vast majority of renders to be based on the data type. So the format/template name should at least be an optional key and when not provided it is determined from the data type.

for reporter in self._reporters:
reporter_handler = self._reporter_handlers.get(reporter.get("backend"))
output_handler = self._output_handlers.get(reporter.get("output"))

Check warning on line 808 in conda/common/io.py

View check run for this annotation

Codecov / codecov/patch

conda/common/io.py#L807-L808

Added lines #L807 - L808 were not covered by tests

if reporter_handler is not None and output_handler is not None:
render_func = getattr(reporter_handler, name, None)

Check warning on line 811 in conda/common/io.py

View check run for this annotation

Codecov / codecov/patch

conda/common/io.py#L811

Added line #L811 was not covered by tests
if render_func is None:
raise AttributeError(

Check warning on line 813 in conda/common/io.py

View check run for this annotation

Codecov / codecov/patch

conda/common/io.py#L813

Added line #L813 was not covered by tests
f"'{name}' is not a valid reporter handler component"
)
render_str = render_func(data, **kwargs)
output_handler.render(render_str, **reporter)

Check warning on line 817 in conda/common/io.py

View check run for this annotation

Codecov / codecov/patch

conda/common/io.py#L816-L817

Added lines #L816 - L817 were not covered by tests


def print_instrumentation_data(): # pragma: no cover
record_file = get_instrumentation_record_file()

Expand Down
87 changes: 86 additions & 1 deletion tests/common/test_io.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
# Copyright (C) 2012 Anaconda, Inc
# SPDX-License-Identifier: BSD-3-Clause
import json
import sys
from io import StringIO
from logging import DEBUG, NOTSET, WARN, getLogger

from conda.common.io import CaptureTarget, attach_stderr_handler, captured
import pytest
from pytest import CaptureFixture

from conda.common.io import (
CaptureTarget,
ConsoleHandler,
JSONHandler,
ReporterManager,
StdoutHandler,
attach_stderr_handler,
captured,
)


def test_captured():
Expand Down Expand Up @@ -80,3 +92,76 @@ def test_attach_stderr_handler():
assert c.stdout == ""
assert "test message" in c.stderr
assert debug_message in c.stderr


def test_console_handler():
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved
"""
Tests the ConsoleHandler ReporterHandler class
"""
test_data = {"one": "value_one", "two": "value_two", "three": "value_three"}
test_str = "a string value"
expected_table_str = "one : value_one\ntwo : value_two\nthree : value_three\n"
console_handler_object = ConsoleHandler()
table_str = console_handler_object.detail_view(test_data)

assert table_str == expected_table_str
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved
assert console_handler_object.string_view(test_str) == test_str


def test_json_handler():
"""
Tests the JsonHandler ReporterHandler class
"""
test_data = {"one": "value_one", "two": "value_two", "three": "value_three"}
test_str = "a string value"
json_handler_object = JSONHandler()
assert json_handler_object.detail_view(test_data) == json.dumps(test_data)
assert json_handler_object.string_view(test_str) == json.dumps(test_str)


def test_std_out_handler(capsys: CaptureFixture):
"""
Tests the StdoutHandler OutputHandler class
"""
test_str = "a string value"
std_out_handler_object = StdoutHandler()
assert std_out_handler_object.name == "stdout"
std_out_handler_object.render(test_str)
stdout = capsys.readouterr()
assert test_str in stdout
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved


def test_reporter_manager(capsys: CaptureFixture):
"""
Tests the ReporterManager class
"""
reporters = (
{"backend": "console", "output": "stdout"},
{"backend": "json", "output": "stdout"},
)
test_data = {"one": "value_one", "two": "value_two", "three": "value_three"}
test_str = "a string value"

# the expected output is output via ConsoleHandler.detail_view plus JsonHandler.detail_view.
expected_detail_view_str = f"one : value_one\ntwo : value_two\nthree : value_three\n{json.dumps(test_data)}"
expected_string_view_str = f"a string value{json.dumps(test_str)}"

reporter_manager_object = ReporterManager(reporters)

# test detail view passes
reporter_manager_object.render("detail_view", test_data)
stdout = capsys.readouterr()
assert expected_detail_view_str in stdout
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved

# test string view passes
reporter_manager_object.render("string_view", test_str)
stdout = capsys.readouterr()
assert expected_string_view_str in stdout
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved

# test fails
with pytest.raises(AttributeError) as err:
reporter_manager_object.render("non_existent_view", test_data)
assert (
str(err.value)
== "'non_existent_view' is not a valid reporter handler component"
)
ForgottenProgramme marked this conversation as resolved.
Show resolved Hide resolved