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

Remove localhost exception #4252

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

chilagrow
Copy link
Contributor

@chilagrow chilagrow commented Apr 23, 2024

Description

Closes #4236.

Readiness checklist

  • I added/updated unit tests (and they pass).
  • I added/updated integration/compatibility tests (and they pass).
  • I added/updated comments and checked rendering.
  • I made spot refactorings.
  • I updated user documentation.
  • I ran task all, and it passed.
  • I ensured that PR title is good enough for the changelog.
  • (for maintainers only) I set Reviewers (@FerretDB/core), Milestone (Next), Labels, Project and project's Sprint fields.
  • I marked all done items in this checklist.

@chilagrow chilagrow added the code/chore Code maintenance improvements label Apr 23, 2024
@chilagrow chilagrow added this to the v1.22.0 milestone Apr 23, 2024
@chilagrow chilagrow self-assigned this Apr 23, 2024
@chilagrow chilagrow requested a review from a team as a code owner April 23, 2024 05:51
@chilagrow chilagrow marked this pull request as draft April 23, 2024 05:52
Copy link

codecov bot commented Apr 23, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 73.95%. Comparing base (7912d98) to head (acc993a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4252      +/-   ##
==========================================
- Coverage   74.51%   73.95%   -0.57%     
==========================================
  Files         339      339              
  Lines       22910    22914       +4     
==========================================
- Hits        17072    16946     -126     
- Misses       4513     4669     +156     
+ Partials     1325     1299      -26     
Files Coverage Δ
integration/setup/listener.go 70.94% <100.00%> (-4.06%) ⬇️
internal/handler/commands.go 100.00% <100.00%> (ø)
internal/handler/authenticate.go 70.83% <57.14%> (-2.70%) ⬇️

... and 18 files with indirect coverage changes

Flag Coverage Δ
filter-true 67.34% <85.71%> (-0.67%) ⬇️
hana-1 ?
integration 67.34% <85.71%> (-0.67%) ⬇️
mongodb-1 5.14% <0.00%> (?)
postgresql-1 46.42% <85.71%> (-0.11%) ⬇️
postgresql-2 49.52% <80.95%> (-0.11%) ⬇️
postgresql-3 49.80% <85.71%> (+0.02%) ⬆️
sqlite-1 45.62% <85.71%> (-0.07%) ⬇️
sqlite-2 48.77% <80.95%> (-0.04%) ⬇️
sqlite-3 48.99% <85.71%> (+0.01%) ⬆️
unit 32.68% <20.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@ -291,13 +291,13 @@ func (h *Handler) Commands() map[string]command {

// authenticateWrapper wraps the command handler with the authentication check.
// If anonymous is true, the command handler is executed without authentication.
func (h *Handler) authenticateWrapper(cmd command) func(context.Context, *wire.OpMsg) (*wire.OpMsg, error) {
func (h *Handler) authenticateWrapper(name string, cmd command) func(context.Context, *wire.OpMsg) (*wire.OpMsg, error) {
Copy link
Member

Choose a reason for hiding this comment

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

It is not quite clear what name is, and the comment wasn't updated

Copy link
Contributor

mergify bot commented May 27, 2024

@chilagrow this pull request has merge conflicts.

@mergify mergify bot added the conflict PRs that have merge conflicts label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/chore Code maintenance improvements conflict PRs that have merge conflicts
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

Remove localhost exception for new auth
2 participants