Skip to content

Conversation

@ykh
Copy link
Contributor

@ykh ykh commented Dec 24, 2018

Description

Use default version (It's latest version as often) when 'version' is not specified by client instead of return 404 error "Invalid version in URL path.".

Closes #6379

version = kwargs.get(self.version_param, self.default_version)

if version is None:
version = self.default_version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be marginally cleaner as:

version = kwargs.get(self.version_param)
if version is None:
    version = self.default_version

Note that we're already checking for "version not in the regex groups", but we're not checking for "version is in the regex groups, but is None".

e.g:
api/users/ > Use Default Version (As Often The Default Version Is The Latest One).
api/v1/users/ > Check 'v1' Is Allowed?
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On blanance I think the intent of the function is clearer from the code itself than it would be from adding this docstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You right, it's unnecessary.

@lovelydinosaur
Copy link
Contributor

lovelydinosaur commented Jan 8, 2019

Fair enough, yup. Thanks. 👍

@lovelydinosaur lovelydinosaur reopened this Jan 8, 2019
@lovelydinosaur lovelydinosaur merged commit 0cf18c4 into encode:master Jan 8, 2019
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020
…d by Client (encode#6380)

* Use Default Version in URLPathVersioning if 'version' Didn't Passed

* Clean Code
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.

2 participants