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

feat(pdk) add kong.node module with '.get_id()' util #3826

Merged
merged 1 commit into from
Oct 4, 2018

Conversation

thibaultcha
Copy link
Member

This new module provides node-level utilities, such as
kong.node.get_id(), which is a replacement to the old
kong.tools.public module's get_node_id() utility.

Reminder: the kong.tools.public module is to be stripped away for the
1.0 stable release.

This new implementation throws errors instead of returning them, for
two reasons:

  1. Simplicity of usage: now that this function is exposed for public
    consumption (and already used by plugins such as Zipkin), avoiding for
    the need to check for errors simplifies its usage.
  2. If such an error occurs, it will be during the initialization of the
    master process, and will then properly prevent Kong from starting. At
    runtime, no such error should occur since the id is cached, but if they
    do, they should be very obvious and provide their stacktraces.

The kong.node module could also contain the following utilities in the
future:

  • get_hostname()
  • get_system_infos()
  • ...

This new module provides node-level utilities, such as
`kong.node.get_id()`, which is a replacement to the old
`kong.tools.public` module's `get_node_id()` utility.

Reminder: the `kong.tools.public` module is to be stripped away for the
1.0 stable release.

This new implementation **throws** errors instead of returning them, for
two reasons:

1. Simplicity of usage: now that this function is exposed for public
consumption (and already used by plugins such as Zipkin), avoiding for
the need to check for errors simplifies its usage.
2. If such an error occurs, it will be during the initialization of the
master process, and will then properly prevent Kong from starting. At
runtime, no such error should occur since the id is cached, but if they
do, they should be very obvious and provide their stacktraces.

The `kong.node` module could also contain the following utilities in the
future:

* get_hostname()
* get_system_infos()
* ...
@thibaultcha thibaultcha merged commit 7fc54f4 into next Oct 4, 2018
@thibaultcha thibaultcha deleted the feat/pdk-node-id branch October 4, 2018 19:09
Copy link
Contributor

@hishamhm hishamhm left a comment

Choose a reason for hiding this comment

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

oh I was writing the review when this was merged

-- @function kong.node.get_id
-- @treturn string The v4 UUID used by this node as its id
-- @usage
-- local id, err = kong.node.get_id()
Copy link
Contributor

Choose a reason for hiding this comment

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

The example says get_id may return a second err argument, but that's not the case. I suggest changing to local node_id = assert(kong.node.get_id()) (no hard feelings either way about the assert, just an idea to convey "should always work")

Copy link
Member Author

Choose a reason for hiding this comment

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

Ugh, a leftover from prior to this change! Thanks!

@thibaultcha
Copy link
Member Author

oh I was writing the review when this was merged

Oops, my bad!

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.

2 participants