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

Catalog Register: Generate UUID for services registered without one #4294

Merged
merged 3 commits into from
Jun 27, 2018

Conversation

mkeeler
Copy link
Member

@mkeeler mkeeler commented Jun 26, 2018

Fixes #4249

This makes the behavior line up with the docs and expected behavior

This makes the behavior line up with the docs and expected behavior
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

Just a couple formatting questions but looks good otherwise.

"github.com/hashicorp/consul/testutil/retry"
"github.com/hashicorp/serf/coordinate"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Is this an artifact of the GitHub diff viewer or are these not tabbed/formatted properly?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right you are, looks like something happened to my editor plugin to run go fmt on save.

return fmt.Errorf("Failed to generate ID: %v", err)
}

args.ID = types.NodeID(id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question here, is this formatted properly (go fmt)?

@mitchellh
Copy link
Contributor

Oh follow up question: should the default UUID test actually go in agent/consul instead of agent (or in addition to) since the logic itself is implemented by the back RPC layer rather than the front agent layer?

@mkeeler
Copy link
Member Author

mkeeler commented Jun 27, 2018

@mitchellh There are actually two catalog_endpoint_test.go files. Looks like I dropped it in the wrong one.

@mkeeler mkeeler force-pushed the bugfix/4249-gen-ext-node-id branch from 08e3bbd to 0f70034 Compare June 27, 2018 13:22
@mkeeler mkeeler merged commit d94747d into master Jun 27, 2018
@mkeeler mkeeler deleted the bugfix/4249-gen-ext-node-id branch July 12, 2018 13:36
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