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

refactor: bash, dockerfile, and CI #121

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pratikbin
Copy link

@pratikbin pratikbin commented Jan 1, 2022

Refactored

  • Merge ca gen bash script to entry point as function
  • Run test every push change in bash script
  • Add gha caching and other CI changes that leads to <58s for cached CI test
  • Refactor bash script as per google style guide

@rpardini
Copy link
Owner

This is, in general, excellent. Thanks.
In the PR workflow, I really wanted two rounds of pulls, and two separate logs, because it's actually testing the cache, so I will rework that back in.
Great tips about the cache mode=gha.
Formatting wise, I will let shellfmt decide, but thanks for all the short-circuiting removals.
I will merge this into a feature branch first, I've a big round of merges to do.

@pratikbin
Copy link
Author

pratikbin commented Jan 11, 2022

In the PR workflow, I really wanted two rounds of pulls, and two separate logs, because it's actually testing the cache, so I will rework that back in.

I will merge this into a feature branch first, I've a big round of merges to do.

Okay.

Question: Have you faced long python compiling time in mitmproxy when creating arm64 image? Because I am! Almost 1100+ seconds.

image

@rpardini
Copy link
Owner

Yes! building arm64 under qemu is ridiculously slow. On real arm64 hardware it's ok, of course. I gotta move the mitmproxy build to the base image https://github.com/rpardini/nginx-proxy-connect-stable-alpine/blob/master/Dockerfile where nginx is already built. Then setup CI there to schedule build every night, even if it's super slow qemu on GHA, we'll never get to experience it.

That would allow for fast(er) arm64 builds of this one, since no building would be done at this level.

@pratikbin
Copy link
Author

I think I should separate changes of this branch in multiple PRs

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.

None yet

2 participants