Skip to content

Conversation

@danielperezz
Copy link
Contributor

@danielperezz danielperezz commented Dec 29, 2025

📝 Description

Add support to adding hub steps to the serving graph by implement infra HubTaskStep and modifying the methods for adding steps to the graph


🛠️ Changes Made

  • Added a HubTaskStep kind, which inherits from TaskStep and implements an init_object which initialize the actual step from the dynamically imported hub step code.
  • Modify params_to_step() by adding another branch to create HubTaskStep in case the class_name parameter is a hub URL.
  • Modify ServingRuntime's deploy() and to_job() methods by adding a step of adding the requirements of the local steps with the requirements of the function
  • Add a lock-step-version utility

✅ Checklist

  • I updated the documentation (if applicable)
  • I have tested the changes in this PR
  • I confirmed whether my changes are covered by system tests
    • If yes, I ran all relevant system tests and ensured they passed before submitting this PR
    • I updated existing system tests and/or added new ones if needed to cover my changes
  • If I introduced a deprecation:

🧪 Testing

New integration test was added to cover the changes in this PR.


🔗 References


🚨 Breaking Changes?

  • Yes (explain below)
  • No

🔍️ Additional Notes

@danielperezz danielperezz requested review from a team and liranbg as code owners December 29, 2025 09:20
Copy link
Contributor

@davesh0812 davesh0812 left a comment

Choose a reason for hiding this comment

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

Looks very good

@pytest.mark.skip(
reason="Remove this marker after steps are added to the functions repo"
)
def test_serving_with_hub_steps(self):
Copy link
Contributor

@davesh0812 davesh0812 Dec 31, 2025

Choose a reason for hiding this comment

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

please lets have the same test also as sys test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Well since it's uses an external resource (hub step) I think that it's enough to use just integ test.

Copy link
Member

@Eyal-Danieli Eyal-Danieli left a comment

Choose a reason for hiding this comment

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

Nice!!

few comments

Copy link
Member

@Eyal-Danieli Eyal-Danieli left a comment

Choose a reason for hiding this comment

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

Nice, please add __init__.py file under the new hub system test dir

Comment on lines 22 to 24
class Echo(BaseClass):
def __init__(self, name=None):
self.name = name
Copy link
Member

Choose a reason for hiding this comment

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

just a recommendation

class Echo(BaseClass):
    def __init__(self, context, name=None):
        super().__init__(context=context, name=name)

Copy link
Member

@Eyal-Danieli Eyal-Danieli left a comment

Choose a reason for hiding this comment

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

just resolve conflicts :)

# Conflicts:
#	mlrun/runtimes/nuclio/serving.py
@Eyal-Danieli Eyal-Danieli merged commit 0297f74 into mlrun:development Jan 12, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants