-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
chore!: remove wdqs-proxy #833
base: main
Are you sure you want to change the base?
Conversation
build/wdqs-frontend/default.conf
Outdated
@@ -23,3 +13,40 @@ server { | |||
root /usr/share/nginx/html; | |||
} | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this optional commented-out section of server config? Does it need a comment or clarification here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, no, that is just a left over. Will be removed. Thanks for catching that.
deploy-lts/docker-compose.yml
Outdated
- "traefik.http.middlewares.wdqs-rate-limit.ratelimit.burst=20" | ||
# Apply middlewares to the wdqs router | ||
- "traefik.http.routers.wdqs-router.middlewares=wdqs-prefix,wdqs-headers,wdqs-cors-headers,wdqs-rate-limit" | ||
# TODO: Set timeouts for the wdqs service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something still requiring attention?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, as I now understand, it is basically handled downstream by the X-BIGDATA-MAX-QUERY-MILLIS
header already. So this would be extra and I would just remove it. Also Traefik serverstransports
configuration seems cursed to me 👻
bases on work done in #767
maybe we should merge and release #828 first, and make this one go together with #812