search

apenella / go-ansible

Go-ansible is a Go package that enables the execution of ansible-playbook or ansible commands directly from Golang applications. It supports a wide range of options for each command, enabling smooth integration of Ansible functionality into your projects.
MIT License
905 stars 143 forks source link

exec.Cmder is internal but needs to be importable from outside #155

Closed robin-rpr closed 2 months ago

robin-rpr commented 3 months ago

Hello there,

First of all thank you very much for this fantastic package!

I wanted to implement go-embed-python into my application that uses go-ansible. There is a good example that you created at https://github.com/apenella/go-ansible/tree/v2.0.0/examples/ansibleplaybook-embed-python

But the example code unfortunately imports a type called exec.Cmder from the internal exec package. Trying out the example mentioned above in my code I realized that I cannot complete my implementation due to the mentioned interface used in the example (and required to define a custom Exectabler) is internal to your package.

It seems to me after major version 2.x of go-ansibe the exec package was made internal.

Is there a workaround to fix this example such that we don't rely on this internal package in it?

If not, I would be happy to contribute a fix, however in that case it would be great to know the drivers for making the exec package internal in go-ansible.

With all the best intentions, Robin =)

Edit: I looked deeper into the source code of this project and now understand why the exec package is internal (as it's a wrapper on the os/exec and since the wrapper is not supposed to be exposed in the package it's internal). However we need to somehow make it accessible to the outside to let people be able to implement a custom Executabler.

apenella commented 3 months ago

Hi @robin-rpr!

Thank you very much for opening this issue! You made a good point here :)

I initially chose not to expose the executable package because, as a wrapper for the exec package, I wanted to keep it separate from the purpose of the library. However, I agree that it could be useful for others to have it available, especially since it is used by the Executabler interface, which is already exposed. So, exposing the executable package is indeed a good suggestion!

I will review your PR and get back to you shortly.

Thanks again!

apenella commented 2 months ago

Changes proposed by @robin-rpr are available on the master branch.

robin-rpr commented 2 months ago

Thank you a lot again @apenella! I just saw your recent activity on this and highly appreciate the help in fixing this 😊