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

plugin: Cannot define parmeterless plugins #2827

Open
LecrisUT opened this issue Apr 4, 2024 · 8 comments
Open

plugin: Cannot define parmeterless plugins #2827

LecrisUT opened this issue Apr 4, 2024 · 8 comments

Comments

@LecrisUT
Copy link
Contributor

LecrisUT commented Apr 4, 2024

Here's an example:

prepare:
  - how: shell
    script: /bin/true
  - how: cmake

The second prepare is not parsed even though I could define defaults for all values

Edit: It seems that adding any default_factory variable makes it work 🤷 (nope)

@happz
Copy link
Collaborator

happz commented Apr 4, 2024

That’s weird. We can see how: tmt in many, many plans, with no extra options, so I’d say the problem will be something else than “cannot define”. Is there an error message? How does debug log looks like, with —log-topic=cli-invocations there should be a very detailed description of phase processing. Can you share the plugin code? Might be related to step_data, but even the default should be good enough.

@LecrisUT
Copy link
Contributor Author

LecrisUT commented Apr 4, 2024

It is in the current iteration of LecrisUT/tmt-cmake#5 (but also on the main branch there). No, no errors are introduced, but also nothing is executed. wake/show are entered, but the actual execution or display does not occur. I'll probably run with debug later

@LecrisUT
Copy link
Contributor Author

LecrisUT commented Apr 4, 2024

Here's a log example running:

prepare:
  how: cmake
#  build_dir: build
execute:
  script: /bin/true

(test/integration/prepare in the plugin repo)

$ tmt run -ra --log-topic=cli-invocations provision --how local
/var/tmp/tmt/run-067
    warn: /plan:prepare - {'how': 'cmake'} is not valid under any of the given schemas

/plan
    discover
        how: shell
        summary: 1 test selected
    provision
        queued provision.provision task #1: default-0
        
        provision.provision task #1: default-0
        how: local
        primary address: localhost
        multihost name: default-0
        arch: x86_64
        distro: Fedora Linux 40 (KDE Plasma Prerelease)
    
        summary: 1 guest provisioned
    prepare
        queued push task #1: push to default-0
        
        push task #1: push to default-0
    
        queued prepare task #1: requires on default-0
        
        prepare task #1: requires on default-0
        how: install
        summary: Install required packages
        name: requires
        where: default-0
        package: /usr/bin/flock
    
        queued pull task #1: pull from default-0
        
        pull task #1: pull from default-0
    
        summary: 1 preparation applied
    execute
        queued execute task #1: default-0 on default-0
        
        execute task #1: default-0 on default-0
        how: tmt
        progress:                 
    
        summary: 1 test executed
    report
        how: display
        summary: 1 test passed
    finish
    
        summary: 0 tasks completed

total: 1 test passed

@lukaszachy
Copy link
Collaborator

Coincidentally I hit the same problem and it is caused by https://github.com/teemtee/tmt/blame/43ac6e339dc4ee8e41a1c8fdcf8f7b91e1e7d0a8/tmt/steps/prepare/__init__.py#L147

Currently trying to remember why such line was added.

Workaround for now can be to define own in plugins dataclass

@dataclasses.dataclass
class PrepareXData(tmt.steps.prepare.PrepareStepData):
    @property
    def is_bare(self):
        return False

@LecrisUT
Copy link
Contributor Author

LecrisUT commented Apr 5, 2024

I think the only reasonable usage is in

tmt/tmt/steps/__init__.py

Lines 1461 to 1468 in 43ac6e3

def show(self, keys: Optional[list[str]] = None) -> None:
""" Show plugin details for given or all available keys """
# Avoid circular imports
import tmt.base
# Show empty config with default method only in verbose mode
if self.data.is_bare and not self.verbosity_level:
return

But over there also I think it should be after the how: is printed, at least if it's not the default how

@psss
Copy link
Collaborator

psss commented Apr 5, 2024

I believe the check for empty data is included in order to drop the default empty phase which is created here:

tmt/tmt/steps/__init__.py

Lines 382 to 384 in 43ac6e3

# Create an empty step by default (can be updated from cli)
if data is None:
raw_data: list[_RawStepData] = [{}]

And that one is created so that there's something which can be overridden from the command line, for example:

tmt run -vvv -a prepare -h install -p tree

When run with the following plan:

provision:
    how: container
execute:
    script: tree

It would be better to create a new phase object only if the respective command line option is provided.

@LecrisUT
Copy link
Contributor Author

LecrisUT commented Apr 5, 2024

You mean that there is effectively a pre-existing:

prepare:
    how: install
    packages: [ ]

In the CLI, doesn't the prepare -h install -p tree effectively add a prepare step anyway? I think it would be better to have such a behavior as opt-in, something like having a property can_skip which can be linked to if self.data.packages. is_bare seems to be more along the line for checking if something was passed to it or not.

@happz
Copy link
Collaborator

happz commented Apr 5, 2024

It's more like a how: something tentative, not really an install incarnation. It's an empty phase into which the default how, i.e. shell, would be injected. The magic is, if it's shell but without any scripts, there's no point to run it, is it? I'll try to poke it, I already wandered around CLI invocations.

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

No branches or pull requests

4 participants