Skip to content

Conversation

@jstirnaman
Copy link
Contributor

@jstirnaman jstirnaman commented Jul 30, 2024

Closes #5513

Related testing and linting changes:

  • Add a Vale spelling config for v2 server options
  • Add influxdb to the test container packages (for testing service influxdb...)
  • Add Dockerfile config and test setup for testing some InfluxDB startup config options (using influxd)

- Fix the http-write-timeout definition, which should be similar to https://pkg.go.dev/net/http#Server.WriteTimeout
- Add a Vale spelling config for v2 server options
- Add influxdb to the test container packages (for testing service influxdb...)
- Add Dockerfile config and test setup for testing some InfluxDB startup config options (using influxd)
@telegraf-tiger telegraf-tiger bot added the fix label Jul 30, 2024
influxd --assets-path=/path/to/custom/assets-dir
```

<!--test-actual
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
<!--test-actual
<!--test:actual

Use the [`influx server-config` command](/influxdb/v2/reference/cli/influx/server-config/)
to retrieve your runtime server configuration.

<!--test: setup
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
<!--test: setup
<!--test:setup

@jstirnaman
Copy link
Contributor Author

@davidby-influx The substantive change fixes the http-write-timeout definition to be similar to https://pkg.go.dev/net/http#Server.WriteTimeout

Copy link

@davidby-influx davidby-influx left a comment

Choose a reason for hiding this comment

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

LGTM

@davidby-influx
Copy link

This change should be for 1.X, as well.

@jstirnaman
Copy link
Contributor Author

This change should be for 1.X, as well.

In v1, is it enqueued-write-timeout or write-timeout?
The migration table says enqueued-write-timeout. Is that correct?

@davidby-influx
Copy link

On looking more closely, it is not present in 1.X, and the given equivalents do something different. In 2.X it is passed to the Go net/http package as a server setting; it 1.X those other two options are handled by Influx code and do slightly different things. So I was wrong it is in 1.x (we don't change the defaults for net/http Server write timeouts.

Let's not put it in 1.X, and I'll look into the equivalency table in more depth later....

@jstirnaman jstirnaman merged commit 41922ad into master Aug 2, 2024
@jstirnaman jstirnaman deleted the jstirnaman/issue5513 branch August 2, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

http-write-timeout documentation is wrong

3 participants