-
Notifications
You must be signed in to change notification settings - Fork 16
fix!: Metrics Service now correctly creates a Population Measurement for a Population Metric #3291
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
fix!: Metrics Service now correctly creates a Population Measurement for a Population Metric #3291
Conversation
a025c90 to
ef163fb
Compare
SanjayVas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SanjayVas reviewed 22 of 22 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @tristanvuong2021)
src/main/k8s/local/BUILD.bazel line 248 at r1 (raw file):
cue_dump( name = "reporting_v2", srcs = ["reporting_v2.cue"],
Drop from srcs since this now comes from deps.
Each file should be in the srcs of no more than one target. I think measurement_system_prober is similarly incorrect and therefore providing a bad example.
src/main/k8s/reporting_v2.cue line 291 at r1 (raw file):
"--port=8443", "--health-port=8080", "--pdp-name=\(_pdpName)",
I believe the references in this file need to be _populationDataProviderName
Suggestion:
_populationDataProviderNamesrc/main/k8s/reporting_v2.cue line 316 at r1 (raw file):
"--port=8443", "--health-port=8080", "--pdp-name=\(_pdpName)",
Suggestion:
_populationDataProviderName
tristanvuong2021
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 20 of 22 files reviewed, 3 unresolved discussions (waiting on @SanjayVas)
src/main/k8s/reporting_v2.cue line 291 at r1 (raw file):
Previously, SanjayVas (Sanjay Vasandani) wrote…
I believe the references in this file need to be
_populationDataProviderName
Done.
src/main/k8s/local/BUILD.bazel line 248 at r1 (raw file):
Previously, SanjayVas (Sanjay Vasandani) wrote…
Drop from srcs since this now comes from deps.
Each file should be in the srcs of no more than one target. I think
measurement_system_proberis similarly incorrect and therefore providing a bad example.
Done.
src/main/k8s/reporting_v2.cue line 316 at r1 (raw file):
"--port=8443", "--health-port=8080", "--pdp-name=\(_pdpName)",
Done.
SanjayVas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SanjayVas reviewed 2 of 2 files at r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @tristanvuong2021)
stevenwarejones
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stevenwarejones reviewed 20 of 22 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @tristanvuong2021)
…e-population-measurement
…e-population-measurement
tristanvuong2021
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tristanvuong2021 reviewed 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @tristanvuong2021)
tristanvuong2021
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tristanvuong2021 reviewed 19 of 22 files at r1, 2 of 2 files at r2, 1 of 1 files at r4, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @tristanvuong2021)
The Metrics Service is fixed to create a RequisitionSpec with population set.
Issue: #3290
BREAKING-CHANGE: The Reporting public API server has a new required
--pdp-nameoption. This is the resource name of the Population Data Provider (PDP) corresponding to the running Population Requisition Fulfiller instance.