Skip to content

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

@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