Added population attributes for yum repositories #84
Conversation
midnightercz
requested review from
jbruzl,
rajulkumar and
rohanpm
as code owners
midnightercz
requested review from
JayZ12138,
chandwanitulsi,
negillett and
rbikar
midnightercz
force-pushed
the
population_sources
branch
2 times, most recently
from
5492dfa to
7dca97f
Compare
midnightercz
force-pushed
the
population_sources
branch
from
7dca97f to
f0ef9a2
Compare
midnightercz
force-pushed
the
population_sources
branch
from
f0ef9a2 to
32e59f3
Compare
| ubi_population = pulp_attrib( | ||
| default=False, type=bool, pulp_field="notes.ubi_population" | ||
| ) | ||
| """Flag indicating whether repo should be populated or not | ||
| """ |
There was a problem hiding this comment.
This change isn't mentioned in the commit msg. Please update it, thx.
rohanpm
left a comment
rohanpm
left a comment
There was a problem hiding this comment.
Could you please also add the new attributes into the schema at pubtools/pulplib/_impl/schema/repository.yaml ?
That schema is partly for documentation but also to make it fail in a controlled way if fields contain data of the wrong type.
| type = pulp_attrib(default="rpm-repo", type=str, pulp_field="notes._repo-type") | ||
|
|
||
| population_sources = pulp_attrib( | ||
| default=(), type=tuple, converter=tuple, pulp_field="notes.population_sources" |
There was a problem hiding this comment.
Can you please make this consistent with the other "immutable collection" attributes, i.e. use FrozenList?
So that would be something like:
| default=(), type=tuple, converter=tuple, pulp_field="notes.population_sources" | |
| default=attr.Factory(FrozenList), type=list, converter=FrozenList, pulp_field="notes.population_sources" |
| ubi_population = pulp_attrib( | ||
| default=False, type=bool, pulp_field="notes.ubi_population" | ||
| ) | ||
| """Flag indicating whether repo should be populated or not |
There was a problem hiding this comment.
population_sources gives the impression of something a bit generic but this one is named specifically relating to UBI. I guess the doc should be slightly more detailed then, like:
| """Flag indicating whether repo should be populated or not | |
| """Flag indicating whether repo should be populated or not for the purposes of UBI |
|
|
||
|
|
||
| def test_populate_attrs(): | ||
| """skip_rsync_repodata is initialized from distributors when possible""" |
There was a problem hiding this comment.
This doc string is wrong, copy-pasted I guess?
midnightercz
changed the title
midnightercz
force-pushed
the
population_sources
branch
from
b4d4553 to
ad092ed
Compare
rohanpm
left a comment
rohanpm
left a comment
There was a problem hiding this comment.
Just realized it's missing a CHANGELOG.md entry too, could you please add something there?
| ubi_population = pulp_attrib( | ||
| default=False, type=bool, pulp_field="notes.ubi_population" | ||
| ) | ||
| """Flag indicating whether repo should be populated or not for the purposes of UBI |
There was a problem hiding this comment.
may mention '...populated from population_sources' just to be consistent with the comment in repository.yaml
Co-Authored-By: Rohan McGovern <rohan@mcgovern.id.au>
- fixed docs strings - added new attributes to schema file
Co-Authored-By: Rohan McGovern <rohan@mcgovern.id.au>
Co-Authored-By: Rohan McGovern <rohan@mcgovern.id.au>
midnightercz
force-pushed
the
population_sources
branch
from
e3054db to
2b8c22d
Compare
Co-Authored-By: Nathan Gillett <negillett@gmail.com>
Co-Authored-By: Rohan McGovern <rohan@mcgovern.id.au>
5 participants
Added 'population_sources' and 'ubi_population'