[WIP] Feat/user decorators v2 #2463
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
romain-intel
force-pushed
the
feat/user_decorators_v2
branch
from
719dcd3 to
5342612
Compare
This also includes packaging for local runs
romain-intel
force-pushed
the
feat/packaging-v2-again
branch
from
301b0d5 to
68ddbcd
Compare
romain-intel
force-pushed
the
feat/user_decorators_v2
branch
from
2f97ddd to
a57a7f1
Compare
romain-intel
force-pushed
the
feat/user_decorators_v2
branch
from
a57a7f1 to
6fb4fbf
Compare
romain-intel
force-pushed
the
feat/user_decorators_v2
branch
from
1436b0c to
38486fc
Compare
Two types of decorators are supported:
- a "pure" Python decorator that allows you to execute something before
and after a step. You also have the possibility of entirely skipping a step
(which is a fairly requested feature)
- mutators (Flow level or Step level). These mutators allow you to modify
the flow/steps dynamically. This is particularly useful when put together
with configs.
Still needs quite a few tests but it should be workable. A few features to try out:
- You should be able to add mutators on a base flow spec class and have it
work in a derived class
- the add_decorator function on the MutableStep should take strings,
actual MF decorators and user decorators
- the add_decorator function on the MutableFlow should take strings
and actual flow level decorators.
- --with should work with any step level decorator (user defined including
step mutators):
- the name is the FQN for the class. It supports passing arguments as well
- you can also use a shorter name provided that name is available *somewhere* where
the flowspec is defined (either itself or one of its base classes)
- mix and match decorators should work
- the UserStepDecorator class has a init function that can take arguments
if you want to build something a bit more fancy
- the mutators also take arguments
- config values can be used for any of those arguments (and of course can be
used inside any of those functions)
- inserted_by is now a list
- decorators are now auto registered on import allowing
use even if not imported in the file; the name of the decorator
is also automatically calculated as the shortest identifying one
- `package info` now shows which decorators are injected by which
portions of code
- added compatibility with pylint for user_step_decorator
Still known:
- configs in arguments to user decorators
- dash in config
Other bug fixes to also make things more natural and just plain old stupid bugs
romain-intel
force-pushed
the
feat/user_decorators_v2
branch
from
38486fc to
5f1e2bd
Compare
…sue with auto import of modules.
saikonen
force-pushed
the
feat/packaging-v2-again
branch
from
c225d81 to
a59c07f
Compare
saikonen
reviewed
Jul 11, 2025
| target_platform = KUBERNETES_CONDA_ARCH or "linux-64" | ||
|
|
||
| def init(self): | ||
| super(KubernetesDecorator, self).init() |
Collaborator
There was a problem hiding this comment.
removed as unnecessary? same question for all the decorators init() changes
Contributor
Author
There was a problem hiding this comment.
why removed? It is necessary. You need to call the base class' init because that is what resolves the attributes.
Contributor
Author
There was a problem hiding this comment.
wait, I removed it? Ok, let me check
Contributor
Author
There was a problem hiding this comment.
Ah right -- I now use this other function called external_init
valayDave
added a commit
to valayDave/metaflow
that referenced
this pull request
Jul 17, 2025
savingoyal
pushed a commit
that referenced
this pull request
Jul 17, 2025
valayDave
added a commit
to valayDave/metaflow
that referenced
this pull request
Jul 22, 2025
When doing something like :
```
from metaflow import project
....
mymutableflow.add_decorator(
project, deco_kwargs={"name":"abc"})
```
Metaflow breaks with the error :
```
File ".../metaflow/flowspec.py", line 312, in _process_config_decorators
deco.pre_mutate(mutable_flow)
File "mydeco.py", line 67, in pre_mutate
mutable_flow.add_decorator(
File ".../metaflow/user_decorators/mutable_flow.py", line 409, in add_decorator
_add_flow_decorator(
File ".../metaflow/user_decorators/mutable_flow.py", line 353, in _add_flow_decorator
d for d in self._flow_cls._flow_decorators if d.name == flow_deco.name
^^^^^^
AttributeError: 'str' object has no attribute 'name'
```
This is because we dont parse the _flow_cls._flow_decorators in the right way (as a dict instead of list)
romain-intel
pushed a commit
that referenced
this pull request
Jul 22, 2025
When doing something like :
```
from metaflow import project
....
mymutableflow.add_decorator(
project, deco_kwargs={"name":"abc"})
```
Metaflow breaks with the error :
```
File ".../metaflow/flowspec.py", line 312, in _process_config_decorators
deco.pre_mutate(mutable_flow)
File "mydeco.py", line 67, in pre_mutate
mutable_flow.add_decorator(
File ".../metaflow/user_decorators/mutable_flow.py", line 409, in add_decorator
_add_flow_decorator(
File ".../metaflow/user_decorators/mutable_flow.py", line 353, in _add_flow_decorator
d for d in self._flow_cls._flow_decorators if d.name == flow_deco.name
^^^^^^
AttributeError: 'str' object has no attribute 'name'
```
This is because we dont handle the _flow_cls._flow_decorators in the
right way (as a dict instead of list)
valayDave
added a commit
to valayDave/metaflow
that referenced
this pull request
Jul 25, 2025
When doing something like :
```
from metaflow import project
....
mymutableflow.add_decorator(
project, deco_kwargs={"name":"abc"})
```
Metaflow breaks with the error :
```
File ".../metaflow/flowspec.py", line 312, in _process_config_decorators
deco.pre_mutate(mutable_flow)
File "mydeco.py", line 67, in pre_mutate
mutable_flow.add_decorator(
File ".../metaflow/user_decorators/mutable_flow.py", line 409, in add_decorator
_add_flow_decorator(
File ".../metaflow/user_decorators/mutable_flow.py", line 353, in _add_flow_decorator
d for d in self._flow_cls._flow_decorators if d.name == flow_deco.name
^^^^^^
AttributeError: 'str' object has no attribute 'name'
```
This is because we dont parse the _flow_cls._flow_decorators in the right way (as a dict instead of list)
valayDave
added a commit
that referenced
this pull request
Aug 11, 2025
`deconame` is not declared and the code would have crashed on that code path. replaced it with the correct variable name
savingoyal
pushed a commit
that referenced
this pull request
Aug 11, 2025
valayDave
added a commit
to valayDave/metaflow
that referenced
this pull request
Aug 11, 2025
- Same flavor of bug fix as Netflix#2513
savingoyal
pushed a commit
that referenced
this pull request
Aug 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.