Skip to content

add weights field for HealthService#1288

Closed
fengxuway wants to merge 2 commits into
hashicorp:masterfrom
fengxuway:fix/master/addweights
Closed

add weights field for HealthService#1288
fengxuway wants to merge 2 commits into
hashicorp:masterfrom
fengxuway:fix/master/addweights

Conversation

@fengxuway
Copy link
Copy Markdown
Contributor

resolve Issue #1287

@hashicorp-cla
Copy link
Copy Markdown

hashicorp-cla commented Sep 25, 2019

CLA assistant check
All committers have signed the CLA.

@fengxuway
Copy link
Copy Markdown
Contributor Author

maybe the test code need change.

Copy link
Copy Markdown

@pierresouchay pierresouchay left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the support of weights

@phamtai97
Copy link
Copy Markdown

Can I ask if the PR has been tested and merged yet?

@fengxuway
Copy link
Copy Markdown
Contributor Author

@phamtai97 Yes. But the test code need compare expected values, so I have to change test code to add Weights field.

See here please: fengxuway@20512ea?diff=split

@eikenb eikenb added this to the 0.25.0 milestone Apr 21, 2020
@eikenb
Copy link
Copy Markdown
Contributor

eikenb commented Apr 21, 2020

I'm going to merge this for 0.25.0 but I'd like the field to be a value, not a pointer. I can make this change post merge but if you get a chance to update the PR, please do.

That is change Weights *api.AgentWeights to Weights api.AgentWeights along with the other required changes.

Thanks.

@pierresouchay
Copy link
Copy Markdown

As a side note, if the value is not set (aka not present on Consul side (ie: à Consul agent pre version 1.0.7 IIRC), the value is expected to be passing:1 and warning:1 (that's the value on Consul side when, for instance DNS retrieve it)

@eikenb
Copy link
Copy Markdown
Contributor

eikenb commented Apr 22, 2020

The conflict is due to formatting. Between that, a new test that was added and needs the Weights and changing it from a pointer to a value... Unless you speak up @fengxuway, I'm just going to pull this down, fix it up and merge from there. I'll be sure to keep attribution on the changes.

@eikenb
Copy link
Copy Markdown
Contributor

eikenb commented Apr 23, 2020

Merged via local checkout.

@eikenb eikenb closed this Apr 23, 2020
@fengxuway fengxuway deleted the fix/master/addweights branch May 18, 2022 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants