WIP: Feature/simon hoenscheid 603 restructure repository management #605
Conversation
|
|
||
| if (($manage_internal_repo or $manage_external_repo) and (($php_version in $external_repo_supported_php_versions) or ($php_version in $os_supported_php_versions))) { | ||
| case $facts['os']['name'] { | ||
| 'Archlinux': { contain 'php::repo::archlinux' } |
There was a problem hiding this comment.
for arch and freebsd, should we just do nothing? since those don't have external repositories.
| 'Archlinux': { contain 'php::repo::archlinux' } | |
| 'Archlinux': { } |
There was a problem hiding this comment.
The classes are for generally managing repos, is there no need to connect a specific one, for a specific php version? Kind of the same problem exists for RHEL 8 style distros, there is the need to activate a specific app stream
There was a problem hiding this comment.
I will implement this
Co-authored-by: Tim Meusel <[email protected]>
| # added for refactoring | ||
| Boolean $manage_external_repo = false, | ||
| Array $external_repo_supported_php_versions = [], | ||
| Hash $external_repo_details = {}, |
There was a problem hiding this comment.
-
should maybe "custom repo detail" or "repo config" or "repo settings" or simply "repository" (is it real relevant if it is a external or internal repository? isn't it "here we have custom repository config" (i can be the default debian repo where i only change a key hash)
-
should be a custom type like
type Profile::Apt::Source = Struct[{
Optional[location] => Optional[String[1]],
Optional[comment] => String[1],
Optional[ensure] => Optional[Enum['file', 'present', 'absent']],
Optional[release] => Optional[String],
Optional[repos] => String[1],
Optional[include] => Optional[Struct[{
Optional[src] => Boolean,
Optional[deb] => Boolean,
}]],
Optional[key] => Optional[Struct[{
Optional[id] => String[1],
Optional[source] => String[1],
}]],
Optional[pin] => Optional[Struct[{
Optional[explanation] => String[1],
Optional[packages] => String[1],
Optional[origin] => String[1],
Optional[priority] => Integer,
}]],
Optional[architecture] => Optional[String],
Optional[allow_unsigned] => Boolean,
Optional[notify_update] => Boolean,
# Optional[comment] => String[1],
# location => 'https://packages.gitlab.com/runner/gitlab-runner/debian/',
# release => 'stretch',
# repos => 'main',
# pin => {
# explanation => 'Prefer GitLab provided packages over the Debian native ones',
# packages => 'gitlab-runner',
# origin => 'packages.gitlab.com',
# priority => 1001,},
# key => {
# 'id' => '1A4C919DB987D435939638B914219A96E15E78F4',
# 'source' => 'https://packages.gitlab.com/runner/gitlab-runner/gpgkey',
# },
# include => {
# 'src' => true,
# 'deb' => true,
# },
}]
There was a problem hiding this comment.
does this work for every repo type?
There was a problem hiding this comment.
"yes" we would have to write a type for each repo stucture we want to support.
There was a problem hiding this comment.
I will add this type to the module
| # defaults: latest stable PHP Version available on php.net, default for OS from Hiera Data in module | ||
| # | ||
| class php::repo ( | ||
| Boolean $manage_internal_repo = $php::manage_internal_repo, |
There was a problem hiding this comment.
not sure if we need internal and external as definition. isn't it simply repository? why we need to differ?
There was a problem hiding this comment.
In my opinion yes, because there are operating systems where we need to manage internal and external repos (RHEL/CENTOS/Fedora) app channel/Remi and if I want to have different data without interpolation this seems to be the way to go
There was a problem hiding this comment.
don't know about centos & co, you manage the internal and the external repo for php?
is it relevant if it is external or internal? isn't it just multiple repositories?
There was a problem hiding this comment.
Its been a while since I worked with Fedora based systems, it only affects RHEL8/CENTOS8 and Fedora IDK which version RHEL8 was branched from +. For me this seems important, because it will prevent users declaring internal and external php versions/sources parallel.
|
Dear @SimonHoenscheid, thanks for the PR! This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs? |
|
I would now remove all old tests so we can rewrite the tests for rewritten classes. So the test status is clear |
Fixes #603