radeklat / delfino

A toolbox of command line helper script, wrapping tools used during Python development.
MIT License
12 stars 3 forks source link

Implement plugin system for commands #32

Closed shimpeko closed 1 year ago

shimpeko commented 2 years ago

closes https://github.com/radeklat/delfino/issues/30

Introduce a plugin system which uses the distribution metadata, entry point, as discovery method.

codecov[bot] commented 2 years ago

Codecov Report

Base: 47.68% // Head: 31.37% // Decreases project coverage by -16.30% :warning:

Coverage data is based on head (c3df436) compared to base (06b571f). Patch coverage: 79.26% of modified lines in pull request are covered.

:exclamation: Current head c3df436 differs from pull request most recent head cefcf25. Consider uploading reports for the commit cefcf25 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #32 +/- ## =========================================== - Coverage 47.68% 31.37% -16.31% =========================================== Files 21 23 +2 Lines 755 816 +61 Branches 161 126 -35 =========================================== - Hits 360 256 -104 - Misses 392 554 +162 - Partials 3 6 +3 ``` | Flag | Coverage Δ | | |---|---|---| | integration_tests | `31.12% <78.04%> (+25.03%)` | :arrow_up: | | total | `31.12% <78.04%> (-16.43%)` | :arrow_down: | | unit_tests | `27.94% <57.31%> (-19.61%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/radeklat/delfino/pull/32?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t) | Coverage Δ | | |---|---|---| | [src/delfino/main.py](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#diff-c3JjL2RlbGZpbm8vbWFpbi5weQ==) | `65.09% <71.73%> (+3.12%)` | :arrow_up: | | [src/delfino/click\_utils/command.py](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#diff-c3JjL2RlbGZpbm8vY2xpY2tfdXRpbHMvY29tbWFuZC5weQ==) | `65.51% <75.00%> (-9.49%)` | :arrow_down: | | [src/delfino/plugin.py](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#diff-c3JjL2RlbGZpbm8vcGx1Z2luLnB5) | `83.33% <83.33%> (ø)` | | | [src/delfino/models/pyproject\_toml.py](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#diff-c3JjL2RlbGZpbm8vbW9kZWxzL3B5cHJvamVjdF90b21sLnB5) | `97.87% <90.90%> (-2.13%)` | :arrow_down: | | [src/delfino/models/command\_package.py](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#diff-c3JjL2RlbGZpbm8vbW9kZWxzL2NvbW1hbmRfcGFja2FnZS5weQ==) | `100.00% <100.00%> (ø)` | | | [src/delfino/click\_utils/filepaths.py](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#diff-c3JjL2RlbGZpbm8vY2xpY2tfdXRpbHMvZmlsZXBhdGhzLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: | | [src/delfino/commands/verify\_all.py](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#diff-c3JjL2RlbGZpbm8vY29tbWFuZHMvdmVyaWZ5X2FsbC5weQ==) | `0.00% <0.00%> (-68.43%)` | :arrow_down: | | [src/delfino/commands/upload\_to\_pypi.py](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#diff-c3JjL2RlbGZpbm8vY29tbWFuZHMvdXBsb2FkX3RvX3B5cGkucHk=) | `0.00% <0.00%> (-53.34%)` | :arrow_down: | | [src/delfino/commands/test.py](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#diff-c3JjL2RlbGZpbm8vY29tbWFuZHMvdGVzdC5weQ==) | `0.00% <0.00%> (-41.56%)` | :arrow_down: | | [src/delfino/commands/lint.py](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t#diff-c3JjL2RlbGZpbm8vY29tbWFuZHMvbGludC5weQ==) | `0.00% <0.00%> (-40.58%)` | :arrow_down: | | ... and [7 more](https://codecov.io/gh/radeklat/delfino/pull/32/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+L%C3%A1t)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

shimpeko commented 2 years ago

@radeklat Can I get your thought on this?

I'd like to change the plugin discovery method to ignore dependent distributions.

The current implementation of plugin discovery, which uses package metadata, discovers plugins from all installed distributions including dependent distributions. For example, when plugin_a depends on plugin_b it will load commands from both plugin_a and plugin_b, even when a user has installed only plugin_a explicitly. And it can cause a naming collision when plugin_a and plugin_b provide commands with the same name.

To solve the above issue I'd like to implement a new plugin discovery approach that uses a list of distributions (plugin names) defined in pyproject.toml (which we've discussed offline). This new plugin approach is not an automatic discovery and users need to list up plugins they want to use in pyproject.toml (in addition to installing the plugin).

With the new plugin discovery approach, I still want to keep using the metadata, to point to package name which contains the command in a plugin.

Another option is using the tool.delfino.disable_plugin_commands to solve command naming collision. But I think it is not ideal to ask users to solve naming collisions caused by dependent distributions (that are installed implicitly).

radeklat commented 1 year ago

@shimpeko Do you have any specific example use case for plugins depending on other plugins? I think that should be discouraged. Perhaps a plugin that would like to enhance a different plugin? For example let's say there is delfino-common plugin that has a command typecheck. And you would like to create a delfino-cookpad-common, which depends on delfino-common, has typecheck that wraps around delfino-common.typecheck and modifies it?

radeklat commented 1 year ago

Merged as part of #42