Add environment variable to override endpoint_url (#2099)

[rwillmer]

1st time contributing to this project, let me know if I need to change anything.

[ozbillwang]

Thanks to raise this PR. I am waiting for this featuer as well (#1375 #2099)

But I have a question.

In latest localstack, it has an improved feature to combine all endpoint url with one port (it used to be different ports for different services). So with your change, it supposes the port is always same for all services.

But will this be always the case?

If in the future, someone need support with different ports, looks this PR hardcode without different port, and it would be hard to update to support different ports for different services.

[hobthross]

I agree @ozbillwang that your suggestion would be an improvement. However since the PR as it stands fixes my use-case, I'm going to leave it as-is, since I don't know my way around the boto3 code well enough to implement it. (More than happy for someone else to take my PR and build on it)

[luminescent]

I am also interested in this PR - it would be very useful for localstack.

[NicolasLeCorre]

We really need this feature, in conjunction with the AWS_CONFIG_FILE and AWS_SHARED_CREDENTIALS_FILE env vars it would allow us to change the cloud backend outside of the code.

but this pr has almost 3 months and no reviewer... is there a problem including it ?

[rwillmer]

Should I ask someone to review it? Not sure what the process is to get a PR into core

[luminescent]

It looks like you did everything right according to their contributions instructions:

https://github.com/boto/boto3/blob/develop/CONTRIBUTING.rst

On Fri, 16 Apr 2021, 19:20 Rachel Willmer, @.***> wrote:

Should I ask someone to review it? Not sure what the process is to get a PR into core

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/boto/boto3/pull/2746#issuecomment-821404693, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJGLDUMBAA5V3S6EJQ5BVDTJB5VRANCNFSM4W2YEVCA .

[luminescent]

If anyone else needs a workaround for localstack while waiting for the PR, I have one using a reverse proxy that intercepts the calls made to the default AWS endpoints and redirects them to localstack on 4566. This means no code changes are required in the lambdas.

https://github.com/luminescent/localstack-reverse-proxy

[CodePint]

+1 this would be incredibly use for me

[rwillmer]

Any chance of getting this merged?

[mojokb]

+1 this would be incredibly use for me

[chrisroat]

This is a very nice, minimally invasive solution to a very common problem. Please do consider it.

[CodePint]

@rwillmer are you able to make the changes requested in the code review? @AidanDelaney I believe this is the reason that this has not been approved?

If you do not have time or I don't hear back, I am happy to branch off and make the requested changes.

[CodePint]

For anyone looking for an interim solution to use with PynamoDB This is the approach I have been using, it is by no means a good one but it allowed me to continue working

import os

from botocore.session import Session
from pynamodb.models import Model


class BaseModel(Model):
    class Meta:
        if os.environ['APP_ENV'] in ['local', 'test']:
            host = os.environ['AWS_ENDPOINT_URL']
            region = os.environ['AWS_DEFAULT_REGION']
        else:
            region = Session().get_config_variable('region')

[eyalengel-pagaya]

watchig. can't wait for this to be merged

[rwillmer]

@AidanDelaney @CodePint I've made the suggested change

[AidanDelaney]

Looks like we need to get the attention of one of the maintainers to see what they think of this contribution. I beleive this PR fixes issue #2099 that was assigned to @kdaily. Maybe they would like to comment and/or review?

[aroxby-honor]

The last two people to merge pull requests were @nateprewitt and @joguSD. Maybe they can offer some guidance on this?

[jbvsmo]

Bumping the issue. It would be greatly appreciated to have this merged.

[rwillmer]

I'm the original author of the PR and AFAIK I've made the suggested change. It's a code change of a few lines, with tests; I cannot understand why it is so hard to get this PR merged.

[aroxby-honor]

@nateprewitt Have you seen this?

[aroxby-honor]

@kdaily Have you seen this?

[JimNero009]

It seems I'm not the first to stumble by this :D

+1, this would make my life stupidly easy.

[CodePint]

@kdaily @nateprewitt @joguSD @sir-sigurd @hugovk Can we please get an update on this and get it merged! If changes need making, say so and they will be made.

If these change will never be accepted, then close the PR. We can then move on with a fork or look for other solutions.

[hugovk]

@CodePint Thanks but I'm not a maintainer here 👍

[CodePint]

ahh sorry @hugovk to bother you, I was just going through people who had got PRs merged recently. If you do have a line of communication open to maintainers I would appreciate you pointing them in our direction!

Thanks

[frantakalab]

+1, really need this <3

[hobthross]

@AidanDelaney @CodePint you are named as the 2 reviewers and this task has been flagged with "Needs Review". Is there anything else you want changed? if not, how do we get this merged before it reaches its 1 year anniversary...

[CodePint]

@hobthross I didnt realise I had been marked as a reviewer. Ideally the try/except block would be simplified but I am happy with it as is and have approved the PR.

Eager to get this merged as well!

[rwillmer]

Great. So does anyone know how we get this merged? I can't do it.

[kapilt]

flagging some folks who have actually approved/merged prs recently @nateprewitt @kyleknap @joguSD @kdaily @stobrien89 ping

This is the most upvoted pr by a wide margin in this repo, and represents a two line change that significantly helps users of this sdk with testing applications.