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

increase burst limit, remove log mounts #4481

Merged
merged 27 commits into from
Feb 16, 2022
Merged

Conversation

shogunpurple
Copy link
Member

Description

A few fixes for running BB in docker self hosted setups:

  • removed needless bind mount for logs
  • Increased burst limit for NGINX

Screenshots

If a UI facing feature, some screenshots of the new functionality.

@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2022

Codecov Report

Merging #4481 (2ebcca8) into master (3aac333) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4481   +/-   ##
=======================================
  Coverage   67.92%   67.92%           
=======================================
  Files         144      144           
  Lines        4923     4923           
  Branches      761      761           
=======================================
  Hits         3344     3344           
  Misses       1105     1105           
  Partials      474      474           
Impacted Files Coverage Δ
...s/server/src/api/controllers/row/internalSearch.js 5.26% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8da7b9...2ebcca8. Read the comment docs.

Copy link
Member

@aptkingston aptkingston left a comment

Choose a reason for hiding this comment

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

LGTM!

@datdamnzotz
Copy link

Just some feed back since I was playing with it this AM,
In my environment I had to dump the built in default config and roll my own

  1. no cert support
  2. Rate limits were low for a couple of my apps.
    Appears to be directly related to how many data providers you have on a page.
    I had to bump it up to 40/s with 88 requests made to the page so with larger apps that load a lot of data (this one is doing 4 graphs on a page) might need to increase it.

image

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2022

CLA Assistant Lite bot:

Thank you for your submission - we really appreciate it ❤️. Like many open-source projects, we ask that you all sign a Contributor License Agreement before we can accept your contribution.

You can sign the CLA by just posting a Pull Request Comment, the same as the text below.

If you are contributing on behalf of a company, your company should contact us to sign a Corporate Contributor License Agreement, via community@budibase.com.


I have read the CLA Document and I hereby sign the CLA


4 out of 5 committers have signed the CLA.
@mike12345567
@shogunpurple
@mslourens
@aptkingston
@Budibase Release Bot
Budibase Release Bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@shogunpurple shogunpurple merged commit d66adab into master Feb 16, 2022
@shogunpurple shogunpurple deleted the fix/docker-things branch February 16, 2022 13:58
@github-actions github-actions bot locked and limited conversation to collaborators Feb 16, 2022
Copy link
Member

@aptkingston aptkingston left a comment

Choose a reason for hiding this comment

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

This is awesome work @shogunpurple 🥳 love it.

hosting/nginx.prod.conf.hbs Show resolved Hide resolved
proxy_pass http://app-service.budibase.svc.cluster.local:4002;
{{#if watchtower}}
location = /v1/update {
proxy_pass http://watchtower-service:8080;
Copy link
Member

Choose a reason for hiding this comment

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

We should use the watchtower variable name here - http://$watchtower:8080;

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants