make JSON_CREDENTIALS optional

ezra-risq commented 1 year ago

The JSON_CREDENTIALS value should be optional.

The google client libraries support alternate authentication methods, so users shouldn't be forced to only use the json credentials file method
My company's security policy forbids the use of json key files stored on compute instances in production, so the json credentials file method is not an option for me
Simply making the JSON_CREDENTIALS option optional allows the underlying Google libraries to just do the right thing as regards the ADC search order (see https://cloud.google.com/docs/authentication/application-default-credentials )
This PR will look for the JSON_CREDENTIALS file if that option is present, and just lets ADC do its thing if not.

meeb commented 1 year ago

Thanks! I've not used anything Google-y for some years so I'm not surprised this needs some maintenance. Your patch looks fine to me if the application credentials detection works properly and as you describe now. Could you just update the readme with a # JSON_CREDENTIALS are optional if you need to override the default credentials detection note or similar comment and I'll merge this into the next release.

(Also, totally unrelated, your SSL cert for https://www.risq.io/ expired in January if you hadn't noticed!).

ezra-risq commented 1 year ago

Great-- I just updated the README.

(Also, totally unrelated, your SSL cert for https://www.risq.io/ expired in January if you hadn't noticed!).

Yeah, my company got acquired and that's not my job anymore-- I have to figure out whose it is; they really should just redirect our old site.

meeb commented 1 year ago

Perfect, thanks!

meeb commented 1 year ago

This has been rolled into the latest release, v3.1.3 and is available now.

micahlagrange commented 7 months ago

This needs done for AWS too, allow us to use instance profiles instead of specifying keys/secrets.

meeb commented 7 months ago

Got an example of using instance profiles with boto? Also that's probably a different issue. I don't personally publish with distill to S3 so I'm happy to accept PRs for that as well.

micahlagrange commented 7 months ago

Just make credentials an optional config and aws tables care if the rest :)

Sent from Gmail Mobile

On Sun, Mar 10, 2024 at 11:05 PM meeb @.***> wrote:

Got an example of using instance profiles with boto? Also that's probably a different issue. I don't personally publish with distill to S3 so I'm happy to accept PRs for that as well.

— Reply to this email directly, view it on GitHub https://github.com/meeb/django-distill/pull/79#issuecomment-1987640693, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADSKUHLEPT7YNFPWBJCBEX3YXU3QNAVCNFSM6AAAAAAWXRTQDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBXGY2DANRZGM . You are receiving this because you commented.Message ID: @.***>

meeb commented 7 months ago

So you can just call boto3.client('s3') with no other arguments and it'll, I assume, pull in the access credentials from the environment?

micahlagrange commented 7 months ago

yep

Sent from Gmail Mobile

On Mon, Mar 11, 2024 at 6:00 AM meeb @.***> wrote:

So you can just call boto3.client('s3') with no other arguments and it'll, I assume, pull in the access credentials from the environment?

— Reply to this email directly, view it on GitHub https://github.com/meeb/django-distill/pull/79#issuecomment-1988278704, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADSKUHOXQMOF6ASUBC6DPWTYXWMG3AVCNFSM6AAAAAAWXRTQDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBYGI3TQNZQGQ . You are receiving this because you commented.Message ID: @.***>

meeb commented 7 months ago

I've created https://github.com/meeb/django-distill/issues/88 to track this as it's a different issue.

meeb / django-distill

make JSON_CREDENTIALS optional #79