chore: Removed deprecated max_loop_allowed argument from Pipeline init (#8409)

* Added equality check for sender and receiver in connection function of pipeline

* Update base.py

irrelevant changes reverted

* added release note

* removed deprecated param max_loops_allowed from pipeline init

* added release note

* revert non relevant test

* Delete releasenotes/notes/remove-support-to-connect-component-to-self-6eedfb287f2a2a02.yaml

* revery non relevant change

* Remove unused test_pipeline_deprecated.yaml

* Remove PipelineMaxLoops error

* Update release notes

---------

Co-authored-by: Silvano Cerza <silvanocerza@gmail.com>
This commit is contained in:
Ajit Singh 2024-09-30 19:28:05 +05:30 committed by GitHub
parent 7ba30d5691
commit 2dd8089409
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 13 additions and 218 deletions

View File

@ -2,11 +2,6 @@
#
# SPDX-License-Identifier: Apache-2.0
import warnings
# TODO: Remove this when PipelineMaxLoops is removed
warnings.filterwarnings("default", category=DeprecationWarning, module=__name__)
class PipelineError(Exception):
pass
@ -28,16 +23,7 @@ class PipelineDrawingError(PipelineError):
pass
class PipelineMaxLoops(PipelineError):
# NOTE: This is shown also when importing PipelineMaxComponentRuns, I can't find an easy
# way to fix this, so I will ignore that case.
warnings.warn(
"PipelineMaxLoops is deprecated and will be remove in version '2.7.0'; use PipelineMaxComponentRuns instead.",
DeprecationWarning,
)
class PipelineMaxComponentRuns(PipelineMaxLoops):
class PipelineMaxComponentRuns(PipelineError):
pass

View File

@ -42,11 +42,6 @@ T = TypeVar("T", bound="PipelineBase")
logger = logging.getLogger(__name__)
_MAX_LOOPS_ALLOWED_DEPRECATION_MESSAGE = (
"'max_loops_allowed' argument is deprecated and will be removed in version '2.7.0'. "
"Use 'max_runs_per_component' instead."
)
class PipelineBase:
"""
@ -58,7 +53,6 @@ class PipelineBase:
def __init__(
self,
metadata: Optional[Dict[str, Any]] = None,
max_loops_allowed: Optional[int] = None,
debug_path: Optional[Union[Path, str]] = None,
max_runs_per_component: int = 100,
):
@ -68,9 +62,6 @@ class PipelineBase:
:param metadata:
Arbitrary dictionary to store metadata about this `Pipeline`. Make sure all the values contained in
this dictionary can be serialized and deserialized if you wish to save this `Pipeline` to file.
:param max_loops_allowed:
How many times the `Pipeline` can run the same node before throwing an exception.
This is deprecated and will be removed in version 2.7.0, use `max_runs_per_component` instead.
:param debug_path:
When debug is enabled in `run()`, where to save the debug data.
:param max_runs_per_component:
@ -91,35 +82,7 @@ class PipelineBase:
)
self._debug_path = Path(debug_path)
if max_loops_allowed is not None:
warnings.warn(_MAX_LOOPS_ALLOWED_DEPRECATION_MESSAGE, DeprecationWarning)
self._max_runs_per_component = max_loops_allowed
else:
self._max_runs_per_component = max_runs_per_component
@property
def max_loops_allowed(self) -> int:
"""
Returns the maximum number of runs per Component allowed in this Pipeline.
This is a deprecated field, use `max_runs_per_component` instead.
:return: Maximum number of runs per Component
"""
warnings.warn(_MAX_LOOPS_ALLOWED_DEPRECATION_MESSAGE, DeprecationWarning)
return self._max_runs_per_component
@max_loops_allowed.setter
def max_loops_allowed(self, value: int):
"""
Sets the maximum number of runs per Component allowed in this Pipeline.
This is a deprecated property, use `max_runs_per_component` instead.
:param value: Maximum number of runs per Component
"""
warnings.warn(_MAX_LOOPS_ALLOWED_DEPRECATION_MESSAGE, DeprecationWarning)
self._max_runs_per_component = value
self._max_runs_per_component = max_runs_per_component
def __eq__(self, other) -> bool:
"""
@ -199,14 +162,8 @@ class PipelineBase:
data_copy = deepcopy(data) # to prevent modification of original data
metadata = data_copy.get("metadata", {})
max_runs_per_component = data_copy.get("max_runs_per_component", 100)
max_loops_allowed = data_copy.get("max_loops_allowed", None)
debug_path = Path(data_copy.get("debug_path", ".haystack_debug/"))
pipe = cls(
metadata=metadata,
max_loops_allowed=max_loops_allowed,
max_runs_per_component=max_runs_per_component,
debug_path=debug_path,
)
pipe = cls(metadata=metadata, max_runs_per_component=max_runs_per_component, debug_path=debug_path)
components_to_reuse = kwargs.get("components", {})
for name, component_data in data_copy.get("components", {}).items():
if name in components_to_reuse:

View File

@ -205,7 +205,6 @@ class Pipeline(PipelineBase):
"haystack.pipeline.input_data": data,
"haystack.pipeline.output_data": final_outputs,
"haystack.pipeline.metadata": self.metadata,
"haystack.pipeline.max_loops_allowed": self._max_runs_per_component,
"haystack.pipeline.max_runs_per_component": self._max_runs_per_component,
},
):

View File

@ -0,0 +1,6 @@
---
upgrade:
- |
Removed `Pipeline` init argument `max_loops_allowed`. Use `max_runs_per_component` instead.
- |
Removed `PipelineMaxLoops` exception. Use `PipelineMaxComponentRuns` instead.

View File

@ -115,7 +115,6 @@ def pipeline_that_has_an_infinite_loop():
component.set_output_types(self, a=int, b=int)
FakeComponent = component_class("FakeComponent", output={"a": 1, "b": 1}, extra_fields={"__init__": custom_init})
pipe = Pipeline(max_loops_allowed=1)
pipe.add_component("first", FakeComponent())
pipe.add_component("second", FakeComponent())
pipe.connect("first.a", "second.x")

View File

@ -56,15 +56,6 @@ class TestPipeline:
It doesn't test Pipeline.run(), that is done separately in a different way.
"""
def test_pipeline_dumps_with_deprecated_max_loops_allowed(self, test_files_path):
pipeline = Pipeline(max_loops_allowed=99)
pipeline.add_component("Comp1", FakeComponent("Foo"))
pipeline.add_component("Comp2", FakeComponent())
pipeline.connect("Comp1.value", "Comp2.input_")
result = pipeline.dumps()
with open(f"{test_files_path}/yaml/test_pipeline.yaml", "r") as f:
assert f.read() == result
def test_pipeline_dumps(self, test_files_path):
pipeline = Pipeline(max_runs_per_component=99)
pipeline.add_component("Comp1", FakeComponent("Foo"))
@ -74,14 +65,6 @@ class TestPipeline:
with open(f"{test_files_path}/yaml/test_pipeline.yaml", "r") as f:
assert f.read() == result
def test_pipeline_loads_with_deprecated_max_loops_allowed(self, test_files_path):
with open(f"{test_files_path}/yaml/test_pipeline_deprecated.yaml", "r") as f:
pipeline = Pipeline.loads(f.read())
assert pipeline.max_loops_allowed == 99
assert pipeline._max_runs_per_component == 99
assert isinstance(pipeline.get_component("Comp1"), FakeComponent)
assert isinstance(pipeline.get_component("Comp2"), FakeComponent)
def test_pipeline_loads_invalid_data(self):
invalid_yaml = """components:
Comp1:
@ -95,7 +78,6 @@ class TestPipeline:
connections:
* receiver: Comp2.input_
sender: Comp1.value
max_loops_allowed: 99
metadata:
"""
@ -114,24 +96,12 @@ class TestPipeline:
connections:
- receiver: Comp2.input_
sender: Comp1.value
max_loops_allowed: 99
metadata: {}
"""
with pytest.raises(DeserializationError, match=".*Comp1.*unknown.*"):
pipeline = Pipeline.loads(invalid_init_parameter_yaml)
def test_pipeline_dump_with_deprecated_max_loops_allowed(self, test_files_path, tmp_path):
pipeline = Pipeline(max_loops_allowed=99)
pipeline.add_component("Comp1", FakeComponent("Foo"))
pipeline.add_component("Comp2", FakeComponent())
pipeline.connect("Comp1.value", "Comp2.input_")
with open(tmp_path / "out.yaml", "w") as f:
pipeline.dump(f)
# re-open and ensure it's the same data as the test file
with open(f"{test_files_path}/yaml/test_pipeline.yaml", "r") as test_f, open(tmp_path / "out.yaml", "r") as f:
assert f.read() == test_f.read()
def test_pipeline_dump(self, test_files_path, tmp_path):
pipeline = Pipeline(max_runs_per_component=99)
pipeline.add_component("Comp1", FakeComponent("Foo"))
@ -143,18 +113,9 @@ class TestPipeline:
with open(f"{test_files_path}/yaml/test_pipeline.yaml", "r") as test_f, open(tmp_path / "out.yaml", "r") as f:
assert f.read() == test_f.read()
def test_pipeline_load_with_deprecated_max_loops_allowed(self, test_files_path):
with open(f"{test_files_path}/yaml/test_pipeline_deprecated.yaml", "r") as f:
pipeline = Pipeline.load(f)
assert pipeline.max_loops_allowed == 99
assert pipeline._max_runs_per_component == 99
assert isinstance(pipeline.get_component("Comp1"), FakeComponent)
assert isinstance(pipeline.get_component("Comp2"), FakeComponent)
def test_pipeline_load(self, test_files_path):
with open(f"{test_files_path}/yaml/test_pipeline.yaml", "r") as f:
pipeline = Pipeline.load(f)
assert pipeline.max_loops_allowed == 99
assert pipeline._max_runs_per_component == 99
assert isinstance(pipeline.get_component("Comp1"), FakeComponent)
assert isinstance(pipeline.get_component("Comp2"), FakeComponent)
@ -288,7 +249,7 @@ class TestPipeline:
# UNIT
def test_repr(self):
pipe = Pipeline(metadata={"test": "test"}, max_loops_allowed=42)
pipe = Pipeline(metadata={"test": "test"})
pipe.add_component("add_two", AddFixedValue(add=2))
pipe.add_component("add_default", AddFixedValue())
pipe.add_component("double", Double())
@ -315,7 +276,7 @@ class TestPipeline:
add_two = AddFixedValue(add=2)
add_default = AddFixedValue()
double = Double()
pipe = Pipeline(metadata={"test": "test"}, max_loops_allowed=42)
pipe = Pipeline(metadata={"test": "test"}, max_runs_per_component=42)
pipe.add_component("add_two", add_two)
pipe.add_component("add_default", add_default)
pipe.add_component("double", double)
@ -344,86 +305,6 @@ class TestPipeline:
}
assert res == expected
# UNIT
def test_from_dict_with_deprecated_max_loops_allowed(self):
data = {
"metadata": {"test": "test"},
"max_loops_allowed": 101,
"components": {
"add_two": {
"type": "haystack.testing.sample_components.add_value.AddFixedValue",
"init_parameters": {"add": 2},
},
"add_default": {
"type": "haystack.testing.sample_components.add_value.AddFixedValue",
"init_parameters": {"add": 1},
},
"double": {"type": "haystack.testing.sample_components.double.Double", "init_parameters": {}},
},
"connections": [
{"sender": "add_two.result", "receiver": "double.value"},
{"sender": "double.value", "receiver": "add_default.value"},
],
}
pipe = Pipeline.from_dict(data)
assert pipe.metadata == {"test": "test"}
assert pipe.max_loops_allowed == 101
assert pipe._max_runs_per_component == 101
# Components
assert len(pipe.graph.nodes) == 3
## add_two
add_two = pipe.graph.nodes["add_two"]
assert add_two["instance"].add == 2
assert add_two["input_sockets"] == {
"value": InputSocket(name="value", type=int),
"add": InputSocket(name="add", type=Optional[int], default_value=None),
}
assert add_two["output_sockets"] == {"result": OutputSocket(name="result", type=int, receivers=["double"])}
assert add_two["visits"] == 0
## add_default
add_default = pipe.graph.nodes["add_default"]
assert add_default["instance"].add == 1
assert add_default["input_sockets"] == {
"value": InputSocket(name="value", type=int, senders=["double"]),
"add": InputSocket(name="add", type=Optional[int], default_value=None),
}
assert add_default["output_sockets"] == {"result": OutputSocket(name="result", type=int)}
assert add_default["visits"] == 0
## double
double = pipe.graph.nodes["double"]
assert double["instance"]
assert double["input_sockets"] == {"value": InputSocket(name="value", type=int, senders=["add_two"])}
assert double["output_sockets"] == {"value": OutputSocket(name="value", type=int, receivers=["add_default"])}
assert double["visits"] == 0
# Connections
connections = list(pipe.graph.edges(data=True))
assert len(connections) == 2
assert connections[0] == (
"add_two",
"double",
{
"conn_type": "int",
"from_socket": OutputSocket(name="result", type=int, receivers=["double"]),
"to_socket": InputSocket(name="value", type=int, senders=["add_two"]),
"mandatory": True,
},
)
assert connections[1] == (
"double",
"add_default",
{
"conn_type": "int",
"from_socket": OutputSocket(name="value", type=int, receivers=["add_default"]),
"to_socket": InputSocket(name="value", type=int, senders=["double"]),
"mandatory": True,
},
)
def test_from_dict(self):
data = {
"metadata": {"test": "test"},
@ -447,7 +328,6 @@ class TestPipeline:
pipe = Pipeline.from_dict(data)
assert pipe.metadata == {"test": "test"}
assert pipe.max_loops_allowed == 101
assert pipe._max_runs_per_component == 101
# Components
@ -508,7 +388,6 @@ class TestPipeline:
def test_from_dict_with_callbacks(self):
data = {
"metadata": {"test": "test"},
"max_loops_allowed": 101,
"components": {
"add_two": {
"type": "haystack.testing.sample_components.add_value.AddFixedValue",
@ -604,7 +483,6 @@ class TestPipeline:
components = {"add_two": add_two, "add_default": add_default}
data = {
"metadata": {"test": "test"},
"max_loops_allowed": 100,
"components": {
"add_two": {},
"add_default": {},
@ -617,7 +495,6 @@ class TestPipeline:
}
pipe = Pipeline.from_dict(data, components=components)
assert pipe.metadata == {"test": "test"}
assert pipe.max_loops_allowed == 100
# Components
assert len(pipe.graph.nodes) == 3
@ -678,7 +555,6 @@ class TestPipeline:
def test_from_dict_without_component_type(self):
data = {
"metadata": {"test": "test"},
"max_loops_allowed": 100,
"components": {"add_two": {"init_parameters": {"add": 2}}},
"connections": [],
}
@ -691,7 +567,6 @@ class TestPipeline:
def test_from_dict_without_registered_component_type(self, request):
data = {
"metadata": {"test": "test"},
"max_loops_allowed": 100,
"components": {"add_two": {"type": "foo.bar.baz", "init_parameters": {"add": 2}}},
"connections": [],
}
@ -702,12 +577,7 @@ class TestPipeline:
# UNIT
def test_from_dict_without_connection_sender(self):
data = {
"metadata": {"test": "test"},
"max_loops_allowed": 100,
"components": {},
"connections": [{"receiver": "some.receiver"}],
}
data = {"metadata": {"test": "test"}, "components": {}, "connections": [{"receiver": "some.receiver"}]}
with pytest.raises(PipelineError) as err:
Pipeline.from_dict(data)
@ -715,12 +585,7 @@ class TestPipeline:
# UNIT
def test_from_dict_without_connection_receiver(self):
data = {
"metadata": {"test": "test"},
"max_loops_allowed": 100,
"components": {},
"connections": [{"sender": "some.sender"}],
}
data = {"metadata": {"test": "test"}, "components": {}, "connections": [{"sender": "some.sender"}]}
with pytest.raises(PipelineError) as err:
Pipeline.from_dict(data)

View File

@ -45,7 +45,6 @@ class TestTracing:
"haystack.pipeline.input_data": {"hello": {"word": "world"}},
"haystack.pipeline.output_data": {"hello2": {"output": "Hello, Hello, world!!"}},
"haystack.pipeline.metadata": {},
"haystack.pipeline.max_loops_allowed": 100,
"haystack.pipeline.max_runs_per_component": 100,
},
trace_id=ANY,
@ -100,7 +99,6 @@ class TestTracing:
operation_name="haystack.pipeline.run",
tags={
"haystack.pipeline.metadata": {},
"haystack.pipeline.max_loops_allowed": 100,
"haystack.pipeline.max_runs_per_component": 100,
"haystack.pipeline.input_data": {"hello": {"word": "world"}},
"haystack.pipeline.output_data": {"hello2": {"output": "Hello, Hello, world!!"}},

View File

@ -64,7 +64,6 @@ def test_default_component_from_dict_unregistered_component(request):
def test_from_dict_import_type():
pipeline_dict = {
"metadata": {},
"max_loops_allowed": 100,
"components": {
"greeter": {
"type": "haystack.testing.sample_components.greet.Greet",

View File

@ -1,14 +0,0 @@
components:
Comp1:
init_parameters:
an_init_param: null
type: test.core.pipeline.test_pipeline.FakeComponent
Comp2:
init_parameters:
an_init_param: null
type: test.core.pipeline.test_pipeline.FakeComponent
connections:
- receiver: Comp2.input_
sender: Comp1.value
max_loops_allowed: 99
metadata: {}