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

Make TeleIRC bridges respect default_branch override #100

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Tjzabel
Copy link
Member

@Tjzabel Tjzabel commented May 15, 2022

TeleIRC bridges running the Go deployment previously only built a single Go binary running the {{ default_branch }}.
This PR updates that to build each repository individually and starts each project with their respective built binary.

closes #99

@Tjzabel Tjzabel requested a review from jwflory May 15, 2022 19:38
@Tjzabel Tjzabel requested a review from a team as a code owner May 15, 2022 19:38
Copy link
Member

@jwflory jwflory left a comment

Choose a reason for hiding this comment

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

I'm wondering if this is the best approach for the role? It makes the Ansible playbook run very slowly since the binary has to be compiled multiple times for each individual bot. I am not sure it is a major advantage to have multiple bridges running on different versions. Maybe I am not seeing the use case?

@@ -9,22 +9,25 @@
- name: git clone/pull RITlug/teleirc
git:
repo: "https://github.com/RITlug/teleirc.git"
dest: "/tmp/teleirc"
version: "{{ default_version }}"
dest: "/tmp/teleirc/{{ item.value.cn }}"
Copy link
Member

Choose a reason for hiding this comment

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

Does /tmp/teleirc/ need to be created first?

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe it creates /tmp/teleirc/ if it doesn't exist yet (pretty sure I blew away that directory during testing).

@Tjzabel
Copy link
Member Author

Tjzabel commented Jun 2, 2022

I'm wondering if this is the best approach for the role? It makes the Ansible playbook run very slowly since the binary has to be compiled multiple times for each individual bot. I am not sure it is a major advantage to have multiple bridges running on different versions. Maybe I am not seeing the use case?

I simply needed a way to be able to test branch changes over several different bridges. I agree I still feel a little wonky doing things this way as I did not merge this PR in yet even though this playbook is currently running in prod. I'm not sure what the best path forward is for testing without doing things in this manner. I thought of only making two binaries, one running off whatever the default is, and one that is the branch to be tested, but i'm not sure how feasible that is to do with ansible.

@jwflory
Copy link
Member

jwflory commented Jun 11, 2022

@Tjzabel I guess the longer arc to this is RITlug/teleirc#343, so we could pluck the pre-built binaries we want from a tagged release. We could keep this Pull Request and git branch open as a working solution, but I think the long arc here is getting the release automation in place so there is less work on building a binary.

What do you think?

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

Successfully merging this pull request may close these issues.

TeleIRC bridges only deploy from {{ default_version }}
2 participants