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: Rename host options to endpoint #3590

Merged
merged 7 commits into from
Aug 28, 2020
Merged

docs: Rename host options to endpoint #3590

merged 7 commits into from
Aug 28, 2020

Conversation

ktff
Copy link
Contributor

@ktff ktff commented Aug 26, 2020

Ref. #1775

This PR address the core of the #1775 and renames host options to endpoint where applicable. Old names are still usable, while the documentation is updated to use endpoint.

This also changes:

  • datadog_logs sink endpoint option into address since the option fits address definition and not endpoint. (Edit: reverted)
  • papertrail sink endpoint option into address for the same reason. (Edit: reverted)
  • pulsar sink address option into endpoint since that option fits endpoint definition.

Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
@ktff ktff added the domain: external docs Anything related to Vector's external, public documentation label Aug 26, 2020
@ktff ktff self-assigned this Aug 26, 2020
@ktff ktff removed the request for review from bruceg August 26, 2020 19:50
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Copy link
Member

@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

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

I think what you have here is good, but just offering that another option for consolidating to just use endpoint would be to use:

The downside is the schemes would be required even though only one is current supported; though I don't think this is different from the pulsar sink and HTTP based sinks probably only really allow for https for the cloud-based services.

@bruceg
Copy link
Member

bruceg commented Aug 27, 2020

I don't see why the datadog_logs and papertrail sinks are being moved away from the endpoint setting. Do they not also use an URL, as evidenced by the use of UriSerde?

Copy link
Member

@bruceg bruceg left a comment

Choose a reason for hiding this comment

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

Looks fine except for the endpoint => address question.

@ktff
Copy link
Contributor Author

ktff commented Aug 27, 2020

why the datadog_logs and papertrail sinks are being moved away from the endpoint setting

datadog_logs and papertrail sinks only use host and port part of UriSerde, which corresponds to address in other sinks, for example socket sink.

Now, that is current situation, but with future upgrades mentioned by @jszwedko it makes more sense to retain them as endpoints.

The downside is the schemes would be required even though only one is current supported

We can supply/assume a default scheme if none is specified.

Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
@ktff ktff requested review from bruceg and jszwedko August 27, 2020 20:57
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: ktf <krunotf@gmail.com>
Signed-off-by: ktf <krunotf@gmail.com>
Signed-off-by: ktf <krunotf@gmail.com>
@ktff ktff merged commit 53c8e00 into master Aug 28, 2020
@ktff ktff deleted the ktff/endpoint_name branch August 28, 2020 13:40
Copy link
Member

@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

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

Apologies, didn't get back to this as quickly as I'd like. This looks good to me. I like your idea of a "default scheme".

mengesb pushed a commit to jacobbraaten/vector that referenced this pull request Dec 9, 2020
* Rename

Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

* Update usages of renamed

Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

* Revert endpoint

Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

* Update tests

Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

* Revert docs

Signed-off-by: ktf <krunotf@gmail.com>

* More test updates

Signed-off-by: ktf <krunotf@gmail.com>
Signed-off-by: Brian Menges <brian.menges@anaplan.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: external docs Anything related to Vector's external, public documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants