Skip to content

chore: populate GITHUB_TOKEN for camoufox fetch in CI#735

Open
Copilot wants to merge 4 commits intomasterfrom
copilot/populate-github-token-envvar
Open

chore: populate GITHUB_TOKEN for camoufox fetch in CI#735
Copilot wants to merge 4 commits intomasterfrom
copilot/populate-github-token-envvar

Conversation

Copy link
Contributor

Copilot AI commented Mar 20, 2026 •
edited by barjin
Loading

Populates the GITHUB_TOKEN envvar to bump the GitHub API rate limits when fetching Camoufox releases. This is read e.g. here.

This should unblock our unstable CI tests.

@barjin
fix: populate GITHUB_TOKEN for camoufox fetch in CI
1b38d79 
Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>
Copilot AI changed the title [WIP] fix: populate GITHUB_TOKEN envvar for camoufox fetch fix: populate GITHUB_TOKEN for camoufox fetch in CI Mar 20, 2026
Copilot AI requested a review from barjin March 20, 2026 12:06
barjin
barjin reviewed Mar 20, 2026
View reviewed changes
Copy link
Member

@barjin barjin left a comment

Choose a reason for hiding this comment

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

@copilot find my review below

@barjin
fix: add default empty value for GITHUB_TOKEN arg and simplify build-…
2d6fbb6 
…arg propagation

Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>
Copilot AI requested a review from barjin March 20, 2026 12:18
@github-actions github-actions bot added this to the 136th sprint - Tooling team milestone Mar 20, 2026
@github-actions github-actions bot added t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics. labels Mar 20, 2026
@barjin barjin added the adhoc Ad-hoc unplanned task added during the sprint. label Mar 20, 2026
@barjin barjin changed the title fix: populate GITHUB_TOKEN for camoufox fetch in CI chore: populate GITHUB_TOKEN for camoufox fetch in CI Mar 20, 2026
barjin
barjin reviewed Mar 20, 2026
View reviewed changes
scripts/test_docker_builds.sh
# Build the image with a unique tag
image_tag="test-$(basename "$template_dir")-$(date +%s)"
if docker build -f "./Dockerfile" -t "$image_tag" . --progress=plain; then
if docker build -f "./Dockerfile" -t "$image_tag" --build-arg GITHUB_TOKEN . --progress=plain; then
Copy link
Member

@barjin barjin Mar 20, 2026

Choose a reason for hiding this comment

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

build-arg is unsuitable for passing secrets, but since build-time secrets are a BuildKit feature (which afaik, isn't a thing on Apify Platform yet), this seems to be our best bet.

Since github.token is ephemeral, it shouldn't harm our testing setup, but caution is required nonetheless.

@barjin barjin marked this pull request as ready for review March 20, 2026 13:18
@barjin barjin requested review from B4nan and vladfrangu March 20, 2026 13:19
B4nan
B4nan approved these changes Mar 20, 2026
View reviewed changes
@vladfrangu
Copy link
Member

vladfrangu commented Mar 20, 2026 •
edited
Loading

Why are these templates still manually fetching camoufox browsers when we have images with it preinstalled? Is this just a case of we forgot to update? 🫠

@barjin
Copy link
Member

barjin commented Mar 20, 2026 •
edited
Loading

Why are these templates still manually fetching camoufox browsers when we have images with it preinstalled

Right, I missed that completely. I'm not sure how / where the original Python Camoufox stores the .mmdb files (maybe that was the reason?), but downloading the browser makes no sense if we have a dedicated base image.

@vladfrangu
Copy link
Member

vladfrangu commented Mar 20, 2026

Also to keep in mind that python-playwright is now a super fat image, so we definitely need to change the templates 👀👀

@barjin
Copy link
Member

barjin commented Mar 20, 2026

Update on this:

  1. Switching to apify/actor-python-playwright-camoufox:3.14-1.57.0 as the base image works.

  2. If we do not call camoufox fetch in the Docker file, the image won't contain ~/.cache/camoufox/geoip with the GeoIP MMDB database (maybe we could add this to the base image?)

  3. If we try to use the GeoIP feature without the database present, Camoufox will download the DB at runtime (this is 60 MBs download on each Actor start, not good). The good news is that this won't crash the Actor.

Possible solutions:

A) Somehow include the GeoIP db in the base image (preferable, but maybe there's a reason why we're not doing this yet).
B) Use apify/actor-python-playwright-camoufox:3.14-1.57.0 and call camoufox fetch in the template anyway. This requires this dubious solution with the GITHUB_TOKEN to make sure our CI is not failing.

@vladfrangu
Copy link
Member

vladfrangu commented Mar 20, 2026 •
edited
Loading

2 surprises me dearly, as we do use camoufox fetch in the base image for it 😓. And its something we absolutely should fix there imo

Edit: https://github.com/apify/apify-actor-docker/blob/31bd92e643d4eb3d3670535ae3d13b06681afe8b/python-playwright-camoufox/Dockerfile#L113

@barjin barjin mentioned this pull request Mar 20, 2026
Merged
@barjin
Copy link
Member

barjin commented Mar 20, 2026

I looked deeper into this, and I believe apify/apify-actor-docker#282 should fix this.

It seems there will be additional age checks in the upcoming Camoufox for Python versions, but we'll cross that bridge when we get there, I guess. For now, this ⬆️ should fix it.

barjin added a commit to apify/apify-actor-docker that referenced this pull request Mar 20, 2026
@barjin
fix: install geoip extra in Python Camoufox image (#282)
bad2dbc 
Installs Camoufox with the `geoip` extra. This causes `camoufox fetch`
to fetch both the browser binary and the `mmdb` database for
geographical IP lookup.

`geoip` is used by default in the [Apify Python Camoufox
template](https://github.com/apify/actor-templates/blob/master/templates/python-crawlee-playwright-camoufox/requirements.txt),
which leads to discrepancies when trying to use the template with the
Docker image.

Related to apify/actor-templates#735
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@barjin barjin barjin left review comments

@B4nan B4nan B4nan approved these changes

@vladfrangu vladfrangu Awaiting requested review from vladfrangu

Assignees

@barjin barjin

Copilot code review Copilot

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Milestone

136th sprint - Tooling team

Development

Successfully merging this pull request may close these issues.

5 participants

@vladfrangu @barjin @B4nan @apify-service-account