-
Notifications
You must be signed in to change notification settings - Fork 18
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
(Fork) Use new Maxmind download URLs and Basic authentication scheme #48
Conversation
Thanks for the contribution! Can you please confirm you've tested it locally? |
Attempted to run the PR checks, however had to cancel https://github.com/runk/node-geolite2/actions/runs/8318151169/job/22760767237?pr=48 after 45 odd minitues of hanging. Any ideas?
Update: Looks like adding account_id didn't make much difference - npm i hangs of the self-install |
The old paths will continue to work with the same parameters. The change is that we will now be redirecting to a presigned R2 URL rather than serving the file directly. If the client is set to follow redirects and the R2 host is not blocked by a firewall rule or proxy, you should be good to go. |
I've been testing locally by running the script directly, and using I'm not sure why the script was hanging after successfully downloading. I've added an explicit |
Yeah, so following redirects was the critical missing piece. I guess if we want to avoid a breaking change to require the account ID, we could continue using the old URL without basic auth? But those old URLs don't appear to be supported, from Maxmind's documentation at least, so this might not work long term. |
I've now made the account ID optional. If not provided, we will continue to use the old URLs without auth, but following redirects. The new URLs will only be used if an account ID is found. Can you try re-running the tests? I think they're now failing only because the license key secret isn't present when a run is triggered by my push. |
Looks like tests fail. I've invited you as a collaborator - you should be able to run tests whenever you like |
The actions still aren't getting access to the secret. It's probably because this PR is off a fork, can you try hitting the "Re-run jobs" button and see if it works when you do it as owner? Otherwise the policy on this repository may have to change. Or we can re-create the PR on a non-fork branch. |
Feel free to create a branch in this repo - I think should should be able
to do it
On Mon, 18 Mar 2024 at 12:08, Simon Garner ***@***.***> wrote:
The actions still aren't getting access to the secret. It's probably
because this PR is off a fork, can you try hitting the "Re-run jobs" button
and see if it works when you do it as owner? Otherwise the policy
<https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#enabling-workflows-for-forks-of-private-repositories>
on this repository may have to change. Or we can re-create the PR on a
non-fork branch.
—
Reply to this email directly, view it on GitHub
<#48 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAG4UBYEFZJRXUSSWOPADILYYZEDXAVCNFSM6AAAAABE2PXULKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBSG42TAMRVGA>
.
You are receiving this because you commented.Message ID:
***@***.***>
--
Cheers,
Dmitry
|
Okay, pushed to a non-fork branch and tests are (almost) working in #52, let's continue discussion there. |
I recently started to see errors indicating
node-geolite2
was unable to download the database files from Maxmind.It appears Maxmind has made some changes in the last few months:
https://dev.maxmind.com/geoip/updating-databases?lang=en#directly-downloading-databases
The permalink URLs to download are different, they now redirect to a presigned URL on
r2.cloudflarestorage.com
, and the initial permalink request requires Basic authentication using the Account ID as username and license key as password, instead of passing the license key as a query parameter.So this PR adds support for:
MAXMIND_ACCOUNT_ID
or from the config asaccount-id