Skip to content

Build maps APK on master#6862

Merged
seadowg merged 5 commits intogetodk:masterfrom
seadowg:map-secrets
Oct 16, 2025
Merged

Build maps APK on master#6862
seadowg merged 5 commits intogetodk:masterfrom
seadowg:map-secrets

Conversation

@seadowg
Copy link
Member

@seadowg seadowg commented Aug 25, 2025

This will set up secrets.properties in build_release where the following env variables are available:

  • GOOGLE_MAPS_API_KEY
  • MAPBOX_ACCESS_TOKEN
  • MAPBOX_DOWNLOADS_TOKEN

@lognaturel as far as I can tell from docs (and from playing with SSH builds and the existingGCLOUD_SERVICE_KEY env variable) we can add these variables in Circle CI and they will only be available to builds running on branches on the main getodk/collect repo.

@seadowg seadowg requested a review from lognaturel August 25, 2025 16:03
@seadowg seadowg marked this pull request as ready for review August 25, 2025 16:03
command: |
if [[ -n "$GOOGLE_MAPS_API_KEY" ]]; then \
echo "GOOGLE_MAPS_API_KEY=$GOOGLE_MAPS_API_KEY" >> secrets.properties
echo "MAPBOX_ACCESS_TOKEN=$MAPBOX_ACCESS_TOKEN" >> secrets.properties
Copy link
Member

Choose a reason for hiding this comment

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

Feels like it would be nicer to have separate conditionals for each of these but I suppose that for our usage it doesn't matter. A fork could presumably want to have only the Mapbox keys.

Copy link
Member Author

Choose a reason for hiding this comment

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

Potentially! They could always mod this the config and then submit a PR back if so.

command: |
if [[ "$CIRCLE_PROJECT_USERNAME" == "getodk" ]]; then \
echo "y" | gcloud beta firebase test android run \
echo "y" | gcloud beta firebase test android run \
Copy link
Member

Choose a reason for hiding this comment

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

Should this also be wrapped in the check for the service key?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, what is the advantage of checking? Should these not just fail if GCLOUD_SERVICE_KEY isn't set? Otherwise, you'd end up with this job passing having done nothing. That would be bad in our situation, but also for a fork that might not realize they're not actually running integration tests.

Copy link
Member

@lognaturel lognaturel left a comment

Choose a reason for hiding this comment

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

Couple of questions that only affect forks. I think the GCloud lack of conditional could be annoying for developer forks but I'm not entirely sure, it depends on whether the job would fail.

@lognaturel
Copy link
Member

Oh, and you haven't added the keys, right? They'll need to be added before we merge this?

@seadowg
Copy link
Member Author

seadowg commented Sep 5, 2025

Oh, and you haven't added the keys, right? They'll need to be added before we merge this?

Yup!

Copy link
Member

@lognaturel lognaturel left a comment

Choose a reason for hiding this comment

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

I have now added the three map-related keys in CircleCI.

@seadowg seadowg merged commit 9d772f7 into getodk:master Oct 16, 2025
6 checks passed
@seadowg seadowg deleted the map-secrets branch October 16, 2025 08:22
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.

2 participants