Implement Internal Metrics Service Read Methods for Reporting V2

[tristanvuong2021]

No description provided.

[wfa-reviewable]

This change is 

[riemanli]

Reviewed 3 of 6 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tristanvuong2021)

---

src/main/kotlin/org/wfanet/measurement/reporting/service/internal/testing/v2/MetricsServiceTest.kt line 1743 at r2 (raw file):

                  width = 0.5f
                }
            }

Isn't this the same as what createCreateMetricRequest provides?

Code quote:

            metricSpec = metricSpec {
              reach =
                MetricSpecKt.reachParams {
                  privacyParams =
                    MetricSpecKt.differentialPrivacyParams {
                      epsilon = 1.0
                      delta = 2.0
                    }
                }
              vidSamplingInterval =
                MetricSpecKt.vidSamplingInterval {
                  start = 0.1f
                  width = 0.5f
                }
            }

[tristanvuong2021]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @riemanli)

---

src/main/kotlin/org/wfanet/measurement/reporting/service/internal/testing/v2/MetricsServiceTest.kt line 1743 at r2 (raw file):

Previously, riemanli (Rieman) wrote…

Isn't this the same as what createCreateMetricRequest provides?

I am trying to be more explicit by putting exactly what I am trying to test in the unit test. The default may be the same, but that isn't known from just looking at the unit test.

[riemanli]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tristanvuong2021)

[stevenwarejones]

Reviewed 3 of 6 files at r1, 1 of 3 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Marco-Premier and @tristanvuong2021)

---

src/main/kotlin/org/wfanet/measurement/reporting/deploy/v2/postgres/PostgresMetricsService.kt line 133 at r3 (raw file):

          .toList()
      } catch (e: IllegalStateException) {
        failGrpc(Status.DATA_LOSS) { "Metric is missing required fields" }

what was the thinking behind using this status code?

---

src/main/kotlin/org/wfanet/measurement/reporting/deploy/v2/postgres/PostgresMetricsService.kt line 160 at r3 (raw file):

        )
      } catch (e: IllegalStateException) {
        failGrpc(Status.DATA_LOSS) { "Metric is missing required fields" }

ditto

---

src/main/kotlin/org/wfanet/measurement/reporting/deploy/v2/postgres/writers/CreateMetric.kt line 329 at r3 (raw file):

      }

    if (existingMetricsMap.size < requests.size) {

why is this if needed?

[tristanvuong2021]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Marco-Premier and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/reporting/deploy/v2/postgres/PostgresMetricsService.kt line 133 at r3 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

what was the thinking behind using this status code?

If some fields that shouldn't be null are somehow null when reading, I want to return early with an exception and this was the only status code I could see matching the issue.

---

src/main/kotlin/org/wfanet/measurement/reporting/deploy/v2/postgres/PostgresMetricsService.kt line 160 at r3 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

ditto

Done.

---

src/main/kotlin/org/wfanet/measurement/reporting/deploy/v2/postgres/writers/CreateMetric.kt line 329 at r3 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

why is this if needed?

If all the metrics have already been created, then the statements would have 0 bindings. Executing those statements would then throw an exception.

[stevenwarejones]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Marco-Premier and @tristanvuong2021)

---

src/main/kotlin/org/wfanet/measurement/reporting/deploy/v2/postgres/PostgresMetricsService.kt line 133 at r3 (raw file):

Previously, tristanvuong2021 (Tristan Vuong) wrote…

If some fields that shouldn't be null are somehow null when reading, I want to return early with an exception and this was the only status code I could see matching the issue.

What about NOT_FOUND?

---

src/main/kotlin/org/wfanet/measurement/reporting/deploy/v2/postgres/writers/CreateMetric.kt line 329 at r3 (raw file):

Previously, tristanvuong2021 (Tristan Vuong) wrote…

If all the metrics have already been created, then the statements would have 0 bindings. Executing those statements would then throw an exception.

should this throw an error then for the else?

[tristanvuong2021]

Reviewable status: 5 of 6 files reviewed, 3 unresolved discussions (waiting on @Marco-Premier, @riemanli, and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/reporting/deploy/v2/postgres/PostgresMetricsService.kt line 133 at r3 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

What about NOT_FOUND?

Done.

---

src/main/kotlin/org/wfanet/measurement/reporting/deploy/v2/postgres/writers/CreateMetric.kt line 329 at r3 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

should this throw an error then for the else?

No, I still need to return the metrics if they were all already created.

[stevenwarejones]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Marco-Premier)