Skip to content

Conversation

@mdboom
Copy link
Contributor

@mdboom mdboom commented Mar 1, 2021

We should block this on mozilla-services/mozilla-pipeline-schemas#661, since this pulls in the new schema from there.

This just adds rate metric type data to the existing RLB schema validation unit test to make sure the output matches the schema.

No CHANGELOG entry since this is purely testing...

@mdboom mdboom requested a review from chutten March 1, 2021 21:19
..Default::default()
},
],
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chutten pointed out in Matrix that maybe we should document how to make these in the developer docs. First, to confirm that this is correct, though...

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks correct to me.

Copy link
Contributor

@chutten chutten left a comment

Choose a reason for hiding this comment

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

There seem to be a lot of changes not related to the rate type in this local schema copy. Where are they from?


let rate_metric: RateMetric = RateMetric::new(CommonMetricData {
name: "rate".into(),
category: "test".into(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put the category first so I can read it in the same order ( test.rate ) that it appears in the payload?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing.

@mdboom
Copy link
Contributor Author

mdboom commented Mar 2, 2021

There seem to be a lot of changes not related to the rate type in this local schema copy. Where are they from?

The vendored copy of the schema hasn't been updated in a while. Something to add to the developer docs...

@mdboom mdboom marked this pull request as ready for review March 2, 2021 13:38
@auto-assign auto-assign bot requested a review from travis79 March 2, 2021 13:38
@mdboom mdboom force-pushed the add-rate-payload-schema branch from 93dae34 to 61598dd Compare March 2, 2021 13:41
@mdboom mdboom requested a review from chutten March 2, 2021 13:50
..Default::default()
},
],
);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks correct to me.

@mdboom mdboom merged commit 78f3a23 into mozilla:main Mar 2, 2021
@mdboom mdboom deleted the add-rate-payload-schema branch March 2, 2021 15:53
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