Skip to content
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

get_default_session() should be public #2707

Open
benkehoe opened this issue Dec 22, 2020 · 7 comments · May be fixed by #3651
Open

get_default_session() should be public #2707

benkehoe opened this issue Dec 22, 2020 · 7 comments · May be fixed by #3651
Labels
feature-request This issue requests a feature. p2 This is a standard priority issue

Comments

@benkehoe
Copy link

benkehoe commented Dec 22, 2020

Currently, DEFAULT_SESSION and setup_default_session() are public members of boto3 module, but the caching _get_default_session() is not. I'd like to see this changed. I often write functions that look like

def my_func(arg1, arg2, session=None):
    if not session:
        session = boto3._get_default_session()

some people write this as

def my_func(arg1, arg2, session=None):
    if not session:
        session = boto3

but this breaks if I'm doing anything other than creating a client.

I can't write the following because it might not be initialized:

def my_func(arg1, arg2, session=None):
    if not session:
        session = boto3.DEFAULT_SESSION

I can't write the following because DEFAULT_SESSION might have been initialized to a custom value already:

def my_func(arg1, arg2, session=None):
    if not session:
        session = boto3.setup_default_session()

I could copy in the logic from _get_default_session() (which only calls setup_default_session() if DEFAULT_SESSION isn't initialized) into each place where I do this, but why make me do that?

There doesn't seem to be any good reason for it to be private, especially given that the fields it operates on are public. I'd ask for the function to be renamed get_default_session() and an alias _get_default_session = get_default_session to be added for backward compatibility.

@benkehoe benkehoe added feature-request This issue requests a feature. needs-triage This issue or PR still needs to be triaged. labels Dec 22, 2020
@benkehoe benkehoe changed the title get_default_session() should be public. get_default_session() should be public Dec 22, 2020
@ghost
Copy link

ghost commented Dec 31, 2020

Here is a temporary solution that you're probably already aware of: (essentially a re-write of _get_default_session())

def my_func(arg1, arg2, session=None):     
    if not session:        
        if boto3.DEFAULT_SESSION is not None:      
            session = boto3.DEFAULT_SESSION      
        else:      
            session = boto3.setup_default_session()     

I'm new to the project and had to do some digging, but it appears that DEFAULT_SESSION is initialized in the setup_default_session() function. If it is not initialized, the first if statement should fail, leading you to the setup_default_session() function without worry of overwriting some already existing custom value.

This is a temporary fix I know, and I'm there with you, _get_default_session() should be public.

Hopefully this helps, please let me know if there's anything I missed!

@benkehoe
Copy link
Author

benkehoe commented Dec 31, 2020

I just use boto3._get_default_session() directly; "private" in Python is a naming convention, not an enforced boundary, and it's very unlikely that this particular function itself would get removed or changed in a breaking way, so I am comfortable relying on it. Making it public (and documented) would indicate to users that there is a standard way to retrieve the cached default session (I'd argue it makes more sense to have a public get_default_session() and setup_default_session() and a private _DEFAULT_SESSION, but that's not a feasible change to make).

@ghost
Copy link

ghost commented Dec 31, 2020

I'm primarily a C programmer, so I didn't even think twice about the mention of private/public! I believe this is a primary reason as to why I love to attempt to assist others on Stack Overflow and GitHub, I end up learning a lot myself! It seems as though private and public is, like you say, for documentation, much like ':' in parameters and '->' in function declarations. I'm curious why the Boto package doesn't use these operators for further documentation? Perhaps the extensive use of pointers makes it unnecessary? Thank you Ben, and my earlier comment has been modified to show the changes I should've noticed myself.

@benkehoe
Copy link
Author

benkehoe commented Jan 3, 2021

Type hints were introduced in Python 3.5 (see PEP 484). boto3 currently still supports Python 2.7, and the code would have a syntax error in 2.7 if there were type hints. While support for Python 3.4 and 3.5 is ending, and Python 2.7 is officially at its end of life, I have not seen that boto3 intends to remove support for it yet.

You can add type annotations in your own code using the 3rd party boto3_type_annotations library.

@swetashre swetashre removed the needs-triage This issue or PR still needs to be triaged. label Jan 5, 2021
@aBurmeseDev aBurmeseDev added the p2 This is a standard priority issue label Nov 11, 2022
@nsandary
Copy link

Is this still being considered?

@benkehoe
Copy link
Author

I'd still love to see it!

@CD-Gamma
Copy link

CD-Gamma commented Nov 9, 2023

I would still love to see this! It's a minor change, but means a lot stylistically in production settings. The change in the linked pull request would not break existing code due to aliasing.

If the feature is not being considered at all, it would be great to hear a rationale or something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request This issue requests a feature. p2 This is a standard priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants