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

docs: Adding clarification that server URL needs to be base URL #189

Merged
merged 1 commit into from
Aug 17, 2020

Conversation

vlastahajek
Copy link
Contributor

Closes #182

Proposed Changes

Adding clarification that server URL needs to be base URL to client API docs and all examples in Readme and API docs

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • Commit messages are in semantic format

Copy link
Contributor

@sranka sranka left a comment

Choose a reason for hiding this comment

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

It is much better than before, I only added a few notes that are up to you to consider valid.

CHANGELOG.md Outdated
@@ -1,3 +1,8 @@
## 2.1.0 [in progress]
### Documentation
1. [#189](https://github.com/influxdata/influxdb-client-go/pull/189) Added clarification that server URL has to the base URL to to API docs and all examples.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. [#189](https://github.com/influxdata/influxdb-client-go/pull/189) Added clarification that server URL has to the base URL to to API docs and all examples.
1. [#189](https://github.com/influxdata/influxdb-client-go/pull/189) Added clarification that server URL has to be the base URL to API docs and all examples.

README.md Outdated
@@ -71,18 +71,18 @@ import (
)

func main() {
// create new client with default option for server url authenticate by token
// Create new client with default option for base server url authenticated by token
Copy link
Contributor

Choose a reason for hiding this comment

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

there is no default option for base server URL, the following comment would probably look better (all files):

Suggested change
// Create new client with default option for base server url authenticated by token
// Create a new client using an InfluxDB base URL and an authentication token

@@ -140,7 +140,7 @@ Client offers two ways of writing, non-blocking and blocking.

### Non-blocking write client
Non-blocking write client uses implicit batching. Data are asynchronously
written to the underlying buffer and they are automatically sent to a server when the size of the write buffer reaches the batch size, default 1000, or the flush interval, default 1s, times out.
written to the underlying buffer and they are automatically sent to a server when the size of the write buffer reaches the batch size, default 5000, or the flush interval, default 1s, times out.
Copy link
Contributor

Choose a reason for hiding this comment

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

good catch 👍

README.md Outdated
@@ -161,7 +161,8 @@ import (
)

func main() {
// Create client and set batch size to 20
// Create new client with default option for base server url authenticated by token
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Create new client with default option for base server url authenticated by token
// Create a new client using an InfluxDB base URL, an authentication token and custom options

client.go Outdated
@@ -79,16 +79,18 @@ type clientImpl struct {
}

// NewClient creates Client for connecting to given serverURL with provided authentication token, with the default options.
// Authentication token can be empty in case of connecting to newly installed InfluxDB server, which has not been set up yet.
// In such case Setup will set authentication token
// serverURL is the base URL, e.g. http://loocalhost:9999,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// serverURL is the base URL, e.g. http://loocalhost:9999,
// serverURL is the InfluxDB base URL, e.g. http://localhost:9999,

@vlastahajek vlastahajek merged commit 0733660 into influxdata:master Aug 17, 2020
@vlastahajek vlastahajek deleted the docs/base_url branch August 17, 2020 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Client does not work with compatibility endpoints
2 participants