Add population attribute annotations #1571
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @jojijac0b)
kungfucraig
requested review from
Marco-Premier and
stevenwarejones
and removed request for
jojijac0b and
Marco-Premier
There was a problem hiding this comment.
just to make sure I'm on the same page:
- the Person message will be what is included as an attribute in the PopulationSpec, correct?
- the population data provider would need to validate that all population_attribute fields are set (hasFoo), correct?
- Is the population data provider expected to check all nested message (recursively check all fields) to see what fields are population_attribute-able? Or just the top-level fields?
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
There was a problem hiding this comment.
- Yes.
- The population data provider will need to verify 1) all fields set by the message in the Any/attribute are annotated with "population_attribute" in the message definition; 2) maybe converse too: all fields in the message that have the "population_attribute" set are present in the Any/attribute message in the population spec.
- We don't expect nesting, just a single level of attributes. @SanjayVas please verify this.
Reviewable status:
There was a problem hiding this comment.
We don't expect nesting, just a single level of attributes. @SanjayVas please verify this.
Correct. This is a restriction on template design, at least in guidelines. Might be enforced by PBM validator.
Reviewable status:
There was a problem hiding this comment.
@jojijac0b fyi
Reviewable status:
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
No description provided.