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

Draft: env variables for ACTION_HISTORY limits #1261

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ Grist can be configured in many ways. Here are the main environment variables it

| Variable | Purpose |
|------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| ACTION_HISTORY_MAX_ROWS | Maximum number of rows allowed in ActionHistory before pruning (up to a 1.25 grace factor). Defaults to 1000. |
| ACTION_HISTORY_MAX_BYTES | Maximum number of rows allowed in ActionHistory before pruning (up to a 1.25 grace factor). Defaults to 1.25Gb. |
| ALLOWED_WEBHOOK_DOMAINS | comma-separated list of permitted domains to use in webhooks (e.g. webhook.site,zapier.com). You can set this to `*` to allow all domains, but if doing so, we recommend using a carefully locked-down proxy (see `GRIST_HTTPS_PROXY`) if you do not entirely trust users. Otherwise services on your internal network may become vulnerable to manipulation. |
| APP_DOC_URL | doc worker url, set when starting an individual doc worker (other servers will find doc worker urls via redis) |
| APP_DOC_INTERNAL_URL | like `APP_DOC_URL` but used by the home server to reach the server using an internal domain name resolution (like in a docker environment). It only makes sense to define this value in the doc worker. Defaults to `APP_DOC_URL`. |
Expand Down
6 changes: 3 additions & 3 deletions app/server/lib/ActionHistoryImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
// on rows and the maximum total number of bytes in the "body" column.
// Pruning is done when the history has grown above these limits, to
// the specified factor.
const ACTION_HISTORY_MAX_ROWS = 1000;
const ACTION_HISTORY_MAX_BYTES = 1000 * 1000 * 1000; // 1 GB.
const ACTION_HISTORY_GRACE_FACTOR = 1.25; // allow growth to 1250 rows / 1.25 GB.
const ACTION_HISTORY_MAX_ROWS = parseInt(process.env.ACTION_HISTORY_MAX_ROWS, 10) || 1000;

Check failure on line 20 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :server-2-of-2:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 20 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :nbrowser-^[O-R]:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 20 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :nbrowser-^[E-L]:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 20 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :nbrowser-^[^A-R]:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 20 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :nbrowser-^[M-N]:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 20 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :server-1-of-2:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 20 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :nbrowser-^[A-D]:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 20 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (:lint:python:client:common:smoke:, 22.x, 3.10)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 20 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :lint:python:client:common:smoke:stubs:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems that the Typescript to do what you want is

Suggested change
const ACTION_HISTORY_MAX_ROWS = parseInt(process.env.ACTION_HISTORY_MAX_ROWS, 10) || 1000;
const ACTION_HISTORY_MAX_ROWS = Number(process.env.ACTION_HISTORY_MAX_ROWS) || 1000;

If what you set in env is NaN, let say "test" it will fallback to default value 1000.

Copy link
Collaborator

@hexaltation hexaltation Oct 14, 2024

Choose a reason for hiding this comment

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

but @fflorent suggestion is better.

const ACTION_HISTORY_MAX_BYTES = parseInt(process.env.ACTION_HISTORY_MAX_BYTES, 10) || 1000 * 1000 * 1000; // 1 GB.

Check failure on line 21 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :server-2-of-2:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 21 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :nbrowser-^[O-R]:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 21 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :nbrowser-^[E-L]:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 21 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :nbrowser-^[^A-R]:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 21 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :nbrowser-^[M-N]:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 21 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :server-1-of-2:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 21 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :nbrowser-^[A-D]:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 21 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (:lint:python:client:common:smoke:, 22.x, 3.10)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.

Check failure on line 21 in app/server/lib/ActionHistoryImpl.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.11, 22.x, :lint:python:client:common:smoke:stubs:)

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing here

Suggested change
const ACTION_HISTORY_MAX_BYTES = parseInt(process.env.ACTION_HISTORY_MAX_BYTES, 10) || 1000 * 1000 * 1000; // 1 GB.
const ACTION_HISTORY_MAX_BYTES = Number(process.env.ACTION_HISTORY_MAX_BYTES) || 1000 * 1000 * 1000; // 1 GB.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Both should work here, I feel like this should make no difference.

const ACTION_HISTORY_GRACE_FACTOR = 1.25; // allow growth to 1.25 times the above limits.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we want to give access to ACTION_HISTORY_GRACE_FACTOR and ACTION_HISTORY_CHECK_PERIOD in env too ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like:

  • not that simple to explain
  • can be dangerous if the value is not controlled

Also YAGNI, so I would leave it as is and let a user either report the need or make a contribution if they really need to

const ACTION_HISTORY_CHECK_PERIOD = 10; // number of actions between size checks.

/**
Expand Down
Loading