Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle local pipeline error propagation #775

Open
PhilippeMoussalli opened this issue Jan 12, 2024 · 4 comments
Open

Handle local pipeline error propagation #775

PhilippeMoussalli opened this issue Jan 12, 2024 · 4 comments

Comments

@PhilippeMoussalli
Copy link
Contributor

PhilippeMoussalli commented Jan 12, 2024

In order to propagate errors that occur in docker back to python, we need to enable abort-on-container-exist flag. However, because we're running containers in sequence, this means that the whole pipeline will abort after the first component has finished executing. One potential solution to resolve this is to run every component individually based on the dependency order as partially implemented here.

class DockerRunner(Runner):

        def run_component(component_name):
            cmd = [
                "docker",
                "compose",
                "-f",
                input_spec,
                "up",
                "--no-deps",
                "--build",
                "--pull",
                "always",
                "--remove-orphans",
                "--abort-on-container-exit",
                f"{component_name}",
            ]

     dependency_order = self.get_service_dependency_order(input_spec)

        if output.returncode != 0:
            msg = f"Command failed with error: '{output.stderr}'"
            raise PipelineRunError(msg)
        # copy the current environment with the DOCKER_DEFAULT_PLATFORM argument
        for component in dependency_order:
            run_component(component)
PhilippeMoussalli added a commit that referenced this issue Jan 12, 2024
Fixes for the 0.9 release: 

* Revert back retires from embedding text component since some models
have their own retry mechanism implemented
[example](https://github.com/langchain-ai/langchain/blob/9b3962fc2521ec0d6ef2ea7c0a40b9c32977671a/libs/community/langchain_community/embeddings/openai.py#L216).
They can be modified through extra arguments
* Reverted back the docker compose error handling since it was blocking
some logs for showing up, the issue should be fleshed out more. Created
a separate ticket for it #775
@RobbeSneyders
Copy link
Member

However, because we're running containers in sequence, this means that the whole pipeline will abort after the first component has finished executing.

I would say this is desirable behavior?

@PhilippeMoussalli
Copy link
Contributor Author

However, because we're running containers in sequence, this means that the whole pipeline will abort after the first component has finished executing.

I would say this is desirable behavior?

Not really, if we set abort-on-container-exist:

  • If something goes wrong in the first component, we can get the error back in python
  • At the same time, the rest of the components in the pipeline won't launch because when the first component finishes executing the whole pipeline is aborted, Unless we implement something like the suggestion above

@RobbeSneyders
Copy link
Member

RobbeSneyders commented Jan 16, 2024

But why would we want to execute the rest of the pipeline if a component has failed? Or does it also not execute if the component succeeds?

@PhilippeMoussalli
Copy link
Contributor Author

But why would we want to execute the rest of the pipeline if a component has failed? Or does it also not execute if the component succeeds?

It does not execute the next component because I think the container-exit counts for both if the component finished successfully or if it exited because of error (so both exit code 0 and 1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

2 participants