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

Return default from hash client when using positional argument #354

Merged
merged 1 commit into from
Oct 15, 2021

Conversation

Pankrat
Copy link
Contributor

@Pankrat Pankrat commented Oct 7, 2021

When using HashClient with ignore_exc, get would always return
None if no server is available and the default is passed as a
positional argument. The other clients return the default
value in this case. An earlier fix only had the desired effect when
passing default as a keyword argument.

For example, Django passes the default as a positional argument.

Return the default value so HashClient behaves like the other
clients even when using

Fixes another variation of issue #350

ChangeLog.rst Outdated Show resolved Hide resolved
@Pankrat
Copy link
Contributor Author

Pankrat commented Oct 8, 2021 via email

When using `HashClient` with `ignore_exc`, `get` would always return
`None` if no server is available and the default is passed as a
positional argument. The other clients return the default
value in this case. An earlier fix only had the desired effect when
passing `default` as a keyword argument.

Return the default value so `HashClient` behaves like the other
clients even when using

Fixes another variation of issue pinterest#350
@Pankrat Pankrat force-pushed the fix-default-hash-client branch from eae77f4 to c2071a9 Compare October 14, 2021 20:34
@jparise jparise merged commit 79d98d2 into pinterest:master Oct 15, 2021
@jparise
Copy link
Collaborator

jparise commented Oct 15, 2021

Thanks @Pankrat!

@bitcity
Copy link

bitcity commented Jan 16, 2022

@Pankrat Just curious if it's better to keep the function parameters (declaration) consistent and have something like :

    def get(self, key, *args, **kwargs):
        if 'default' in kwargs:
            default = kwargs.get('default')
        elif args:
            default = args[0]
        else:
            default = None
        return self._run_cmd('get', key, default, *args, **kwargs)

as opposed to

    def get(self, key, default=None, **kwargs):

jparise pushed a commit that referenced this pull request Feb 20, 2022
When using HashClient with ignore_exc, get would always return
None if no server is available and the default is passed as a
positional argument. The other clients return the default
value in this case. An earlier fix only had the desired effect when
passing default as a keyword argument.

For example, Django passes the default as a positional argument.

Return the default value so HashClient behaves like the other
clients.
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.

4 participants