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

[TODO: remove in v15] Duplicate HTTP Auth Basic and HTTP Auth Digest tools have to be merged #1688

Closed
webknjaz opened this Issue Jan 13, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@webknjaz
Member

webknjaz commented Jan 13, 2018

While working on #1680 I realized that there's functional duplication in tools we provide by default.
There's auth_basic, auth_digest and also basic_auth, digest_auth.
They also have duplicate tests spread across 3-5 test modules, which has to be handled as well.

#913 (0704e6c) and as mentioned in #914:

Note that with this new auth_digest and the auth_basic in ticket #913, the files lib/auth.py and lib/httpauth.py can be completely deprecated.

  • I'm submitting a ...
  • bug report
  • feature request
  • question about the decisions made in the repository
  • WAT
  • Do you want to request a feature or report a bug?
    Deprecated tool removal.

  • What is the current behavior?
    There's ambiguity making end-users confused about which tool to use.

  • If the current behavior is a bug, please provide the steps to reproduce and if possible a screenshots and logs of the problem.
    See src

  • What is the expected behavior?
    Just one well-documented and well-tested implementation for each of Basic/Digest HTTP Auth methods.

  • What is the motivation / use case for changing the behavior?
    There should be one-- and preferably only one --obvious way to do it. (Zen of Python)

  • Please tell us about your environment:

  • Cheroot version: —
  • CherryPy version: 3.2.0‒13.1.0+
  • Python version: —
  • OS: —
  • Browser: —
  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, e.g. stackoverflow, gitter, etc.)

@webknjaz webknjaz added the task label Jan 13, 2018

@jaraco jaraco self-assigned this Jan 13, 2018

jaraco added a commit that referenced this issue Jan 13, 2018

jaraco added a commit that referenced this issue Jan 13, 2018

jaraco added a commit that referenced this issue Jan 13, 2018

jaraco added a commit that referenced this issue Jan 13, 2018

jaraco added a commit that referenced this issue Feb 4, 2018

jaraco added a commit that referenced this issue Feb 4, 2018

jaraco added a commit that referenced this issue Feb 4, 2018

@webknjaz webknjaz changed the title from Duplicate HTTP Auth Basic and HTTP Auth Digest tools have to be merged to [TODO: remove in v15] Duplicate HTTP Auth Basic and HTTP Auth Digest tools have to be merged Apr 22, 2018

@webknjaz webknjaz self-assigned this Apr 22, 2018

@webknjaz

This comment has been minimized.

Member

webknjaz commented May 21, 2018

@jaraco we've forgotten to remove this thing. Maybe we need some label for deprecations to check before doing major releases?

@webknjaz

This comment has been minimized.

Member

webknjaz commented May 21, 2018

The only thing I worry about is that new tool runs on before_handler, while the old one was used to be running on on_start_resource.

@webknjaz webknjaz closed this in 38ad1da May 21, 2018

@jaraco

This comment has been minimized.

Member

jaraco commented May 21, 2018

At YouGov, we have some applications that still use the deprecated handlers. I'm not sure we've given enough time for deprecation warnings to be noticed and addressed.

Also, it's not that we had forgotten to remove it; we just hadn't yet removed it. I don't think there's any prescribed schedule for removing deprecated functionality.

@webknjaz

This comment has been minimized.

Member

webknjaz commented May 21, 2018

Well, I removed those in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment