Implement plugin/modular system for FMF metadata loaders (Phase 1) - #303
Implement plugin/modular system for FMF metadata loaders (Phase 1)#303jscotka wants to merge 1 commit into
Conversation
Add a plugin architecture to support reading metadata from multiple file formats beyond .fmf files. This enables future support for bash scripts, Python/pytest tests, and other formats while maintaining full backward compatibility. ## Architecture ### Core Components **fmf/plugin.py** (148 lines): - Abstract Plugin base class - Methods: can_handle(), read(), write() - Attributes: extensions, file_patterns, priority (0-200) - Helper: _write_fmf_fallback() for plugins that can't write natively **fmf/plugin_loader.py** (73 lines): - PluginRegistry for managing built-in plugins - Static registration (no dynamic loading - security) - Priority-based plugin selection - Config validation against known plugin names **fmf/plugins/__init__.py** (23 lines): - Static registration of all built-in plugins - PLUGIN_NAMES dict for config validation - Single source of truth for available plugins **fmf/plugins/fmf.py** (110 lines): - FmfPlugin - refactored .fmf YAML loading - Uses ruamel.yaml for consistency - Priority 100 (default format) - Full write support via dict_to_yaml() ### Tree Integration **fmf/base.py**: - Import fmf.plugins to trigger registration - Re-export MAIN, SUFFIX for backward compatibility - _initialize(): Load plugins from config - grow(): Use get_plugin_for_file() for each file - __exit__(): Plugin write support in context manager ### Configuration .fmf/config format: ## Features ✅ **Static registration** - All plugins in fmf/plugins/__init__.py ✅ **Priority system** - 0-200 scale, configurable per plugin ✅ **Priority override** - Adjust via config ✅ **Security** - Only built-in plugins allowed ✅ **can_handle()** - Direct filtering, supports regex patterns ✅ **Write fallback** - _write_fmf_fallback() helper ✅ **Backward compatible** - 100% existing test pass ✅ **Mixed formats** - .fmf and other types in same tree ## Testing **tests/unit/test_plugin.py** (29 tests, 644 lines): - TestPluginRegistry (7 tests) - Registration, validation - TestFmfPlugin (5 tests) - Read, write, can_handle - TestTreeWithPlugins (6 tests) - Tree integration - TestPluginConfigurationOverride (3 tests) - Priority override - TestRealWorldExamples (2 tests) - Existing examples - TestMockPlugin (6 tests) - Multi-format with .txt files **Test data**: - tests/unit/data/plugin_basic/ - Config and .fmf files **Coverage**: All 275 tests pass ## Simplifications Made 1. **plugin_loader.py**: 169 → 73 lines (57% reduction) - Removed dynamic loading (importlib, inspect) - Removed get_supported_file_patterns() (unused) - Simplified to pure tracking + validation 2. **Static registration**: No environment variables, no file paths - All plugins registered in fmf/plugins/__init__.py - Config just validates plugin names - Security-focused design 3. **Direct filtering**: Use can_handle() not pre-filtering - Let plugins decide what they handle - Supports regex patterns in file_patterns - No verbose logging for non-matches ## Documentation **docs/concept.rst**: - New Plugins section - Configuration examples - Priority override documentation - Security model explanation **PLUGIN_FUTURE.md** (592 lines): - Phase 2: Bash plugin design - Phase 3: Python/pytest plugin design - Phase 4: Write-back support - Implementation steps (no code) - Configuration system - Testing strategy - Security model - Migration guide ## Backward Compatibility ✅ Trees without plugin config work (FmfPlugin auto-loaded) ✅ All existing .fmf files load correctly ✅ SUFFIX and MAIN constants still available ✅ All 267 existing tests pass unchanged ✅ No breaking changes to Tree API ## Security Model 🔒 **Only built-in plugins** from fmf/plugins/ directory 🔒 **No dynamic loading** from environment or arbitrary paths 🔒 **Static registration** in fmf/plugins/__init__.py 🔒 **Config validation** against PLUGIN_NAMES 🔒 **No code execution** (future Python plugin uses AST only) ## Future Phases **Phase 2**: BashPlugin - Read from # fmf-key: value comments **Phase 3**: PytestPlugin - Extract from marks and docstrings **Phase 4**: Enhanced write-back support ## Files Changed New files: - fmf/plugin.py (148 lines) - fmf/plugin_loader.py (73 lines) - fmf/plugins/__init__.py (23 lines) - fmf/plugins/fmf.py (110 lines) - tests/unit/test_plugin.py (644 lines) - tests/unit/data/plugin_basic/* (4 files) - PLUGIN_FUTURE.md (592 lines) Modified files: - fmf/base.py (minimal changes for plugin integration) - docs/concept.rst (added Plugins section) Total: ~1590 lines added ## Contributors Based on: - Original issue: #103 - POC branch: py_plugin - Design discussions and iterations Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
45ca238 to
b6b0a8a
Compare
LecrisUT
left a comment
There was a problem hiding this comment.
Some initial design comments
| Plugins | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| Starting with fmf 2.0, a plugin system allows reading metadata from |
There was a problem hiding this comment.
Why 2.0? That would need a whole lot of other discussion rounds and gathering cleanups to do. For now it is additive so can skip that.
Also there are proper sphinx directives to indicate which version it targets
There was a problem hiding this comment.
I've also though about that. and for me it also seems good idea to increase version, as this is significant change and probably we should also change version of .fmf/version to 2.0
But theoretically it does not matter so much, as it is backward compatible. So yes, we can remove this part at all if you want.
There was a problem hiding this comment.
Agreed on last hacking session to keep 1.x for now
|
|
||
| * FmfPlugin: 100 (default format) | ||
| * BashPlugin: 50 (future) | ||
| * PytestPlugin: 50 (future) |
There was a problem hiding this comment.
Why is the priority even needed?
There was a problem hiding this comment.
For me it seems more robust and flexibile, as FMF should have higest prio when loading data, and bash and python files as well. I've expected that someone can make it overlapping. so that this priority can make plugin handling more flexible. e.g. imagine situation, that someone decide to rewrite some testsuite from bash to python, and will have old bash code here with new tests (with same tests and same naming) causes that if bash will have higher prio by default, override python metadata.
I understand that this is probably corner case, and we can/couldn't support this scenario at all.
Discussion is welcomed. I can remove also this part as not crucial. Theoretically we can also add it later on demand
There was a problem hiding this comment.
Iiuc the only difference is if you have both an fmf and pytest/bash node, which order does the equivalent of elasticity feature resolve? We do not have it well-defined there either :/
But I would rather we well-define the order and let the rest be a user issue until we get some context on how a user would prefer. I do not have strong preference if fmf files should be on top of pytest/bash, I would mostly use it as additive to each other. The only thing that comes to mind are the /: or adjust features, which should be in fmf files only IMO, and for that probably fmf definition being on the bottom would be needed?
There was a problem hiding this comment.
yes, this is completely right, that it is not well defined as well now, e.g. use main.fmf with key /a/b/c and then /a/b/c/main.fmf so that they are definig same item on same level twice. We've tried to solve in past as I remember some discussion but without success, or better to say no decision :-)
Yes, We can do some sort of harcoded priority, probably better than this flexibility.
For FMF files It is little bit harder, there are two points, first is that main.fmf will have still lowest prio, as this creates hierarchy (I do not expect to use some main.sh, or main.py to be used for same thing) but still as the most flexible and cleanest format it should be able to override/append data into another types. It is connected with my idea that if some plugin/format will not support writing data back to the source code (hard to do or create crapp in source code), the easiest way is to overrride them with fmf file + items, so that most of data will live with tests and then e.g. generated IDs could be part or fmf files for same level of data.
There was a problem hiding this comment.
I've forgot to mention also main idea about that priority, what was in my mind :-)
I plan to write two python plugins, one will support using just doc strings (for project what do not want to be dependent on FMF project using annotaded yaml strip inside) and second one with decorators like some @FMF.author("A B") -- and in this case I cannot decide situation when somebody wants to use both together.
@cle what do you think in this case. also hardcode the prio based on my decision? Or allow to use both ways inside one plugin together, so that, decision and documentation will be then on this plugin itself (but this part could cause some nondeterministic behaviour where to write data back into files, theoretically could be also defined inside documentation for plugin in case it will be part of one plugin)
There was a problem hiding this comment.
as this creates hierarchy (I do not expect to use some main.sh, or main.py to be used for same thing)
I do, for example
# test_someting.py
"""
.. fmf::
duration: 10m
"""
def normal_test(): ...
def long_test(): ...
"""
.. fmf::
duration+: "* 10"
"""@cle what do you think in this case. also hardcode the prio based on my decision? Or allow to use both ways inside one plugin together, so that, decision and documentation will be then on this plugin itself (but this part could cause some nondeterministic behaviour where to write data back into files, theoretically could be also defined inside documentation for plugin in case it will be part of one plugin)
I would say they should be mutually exclusive.
As for the order itself, if we do not resolve the order issue in fmf files itself, I think we can assume for now that the user will play nice and not override willy-nilly. Or at least make it bright red in the documentation that this is a yet to be defined behavior.
PS: poor @cle being pingedout of the blue.
There was a problem hiding this comment.
Agreed on last hacking session to not have it user-defined. There were no strong opinions if fmf should be on top of python or vice-versa, so fine to make a case for either one. Similar with the elasticity we do not have a well-documented behavior and just hope the user doesn't override it, but we should document what the current method is for elasticity and similarly whatever we go for here.
| # Register built-in plugins | ||
| _registry = get_registry() | ||
| _registry.register(FmfPlugin) | ||
|
|
There was a problem hiding this comment.
With a new implementation it should use modern designs such as using entry-point. When starting from scratch here would not even bother providing many ways to define these, not even a hard-coded initial dict
There was a problem hiding this comment.
I'll rework it. I've never used this entry-point so didn't know it exists :-)
There was a problem hiding this comment.
From hacking session, only one method and entry-point can cover some more complex edge-cases, so let's go with it.
| FileError: If file cannot be parsed or contains duplicate keys | ||
| """ | ||
| try: | ||
| with open(filename, encoding='utf-8') as datafile: |
There was a problem hiding this comment.
yes I can change it, it is older relict from old past implementation.
Plugin/Modular System for FMF Metadata Loaders (Phase 1)
Implements a plugin architecture to support reading metadata from multiple file formats beyond
.fmffiles.Key Features
Pluginbase class,PluginRegistry, static registrationfmf/plugins/, no dynamic loadingConfiguration
Testing
Files Added
Total: 1768 lines added, 20 lines modified
Next Steps
Closes #103