Skip to content
This repository was archived by the owner on Sep 22, 2023. It is now read-only.

feat: update tunnels in demo to use simplified service #106

Merged
merged 3 commits into from
Nov 3, 2021

Conversation

dbluhm
Copy link
Contributor

@dbluhm dbluhm commented Oct 25, 2021

The upstream agent-tunnel service has been updated to make things simpler and more reliable. We now pull from the wait script available in that repo and just wrap with setting the ACA-Py environment variable. This should help us to consume improvements from upstream in the future more easily.


COPY --chown=indy demo/agent-tunnel-wait.sh ./agent-tunnel-wait.sh
RUN chmod +x agent-tunnel-wait.sh
ADD --chown=indy https://raw.githubusercontent.com/Indicio-tech/agent-tunnel/02351dd87e105a7522a2d752ddd174a52a6755a1/wait-curl.sh ./wait.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a specific reason for this to be pointing at a specific commit, if the purpose of this PR is to make things easier to follow upstream's changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair question. By pointing to a specific commit, we make pulling updates from upstream intentional but still trivial. If we want updated scripts, just grab the new commit. In future, I would like to have this point to a release instead of a commit but this will function essentially the same way.

ENTRYPOINT ["/bin/bash", "-c", "./agent-tunnel-wait.sh \"$@\"", "--"]
CMD ["poetry", "run", "aca-py", "start", "--plugin", "acapy_plugin_toolbox", "--arg-file", "configs/default.yml"]
ENTRYPOINT ["/bin/bash", "-c", "./acapy-endpoint.sh poetry run aca-py \"$@\"", "--"]
CMD ["start", "--arg-file", "configs/default.yml"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the toolbox plugin automatically detected now? I see that the --plugin acapy_plugin_toolbox argument has been removed, and it does not appear to be in the compose files as well.

Based on the fact that the compose files did not have such an argument before, I believe that it is fine to remove the plugin argument. I'm assuming that this is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The plugin option was already in the config file. It's presence in the command was redundant so I dropped it.

Copy link
Contributor

@TheTechmage TheTechmage left a comment

Choose a reason for hiding this comment

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

Alright, I'm satisfied and these changes then. It would be good to have something more up-to-date

@dbluhm dbluhm enabled auto-merge October 27, 2021 20:48
@dbluhm dbluhm disabled auto-merge November 3, 2021 18:26
@dbluhm dbluhm merged commit 534da74 into hyperledger-aries:main Nov 3, 2021
@dbluhm dbluhm deleted the feature/update-tunneling branch November 3, 2021 18:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants