Skip to content

(CONT-361) Syntax update #1397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 24, 2023
+394 −395
Merged

(CONT-361) Syntax update #1397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
655ac5e
(CONT-361) Syntax update
LukasAud Feb 8, 2023
e522f51
Parameter documentation rule
LukasAud Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .puppet-lint.rc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
--relative
--no-parameter_types-check
--no-parameter_documentation-check
--no-anchor_resource-check
--no-params_empty_string_assignment-check
2 changes: 0 additions & 2 deletions .sync.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,5 @@ spec/spec_helper.rb:
delete: true
Rakefile:
extra_disabled_lint_checks:
- parameter_types
- parameter_documentation
- anchor_resource
- params_empty_string_assignment
3 changes: 0 additions & 3 deletions Rakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,9 @@ def changelog_future_release
end

PuppetLint.configuration.send('disable_relative')
PuppetLint.configuration.send('disable_parameter_types')
PuppetLint.configuration.send('disable_parameter_documentation')
PuppetLint.configuration.send('disable_anchor_resource')
PuppetLint.configuration.send('disable_params_empty_string_assignment')


if Bundler.rubygems.find_name('github_changelog_generator').any?
GitHubChangelogGenerator::RakeTask.new :changelog do |config|
raise "Set CHANGELOG_GITHUB_TOKEN environment variable eg 'export CHANGELOG_GITHUB_TOKEN=valid_token_here'" if Rake.application.top_level_tasks.include? "changelog" and ENV['CHANGELOG_GITHUB_TOKEN'].nil?
Expand Down
52 changes: 26 additions & 26 deletions manifests/backup/pg_dump.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@
# Manage creation of the backup user.
# @param optional_args
# Specifies an array of optional arguments which should be passed through to the backup tool. These options are not validated, unsupported options may break the backup.
# @param postscript
# @param post_script
# One or more scripts that are executed when the backup is finished. This could be used to sync the backup to a central store.
# @param prescript
# @param pre_script
# One or more scripts that are executed before the backup begins.
# @param rotate
# Backup rotation interval in 24 hour periods.
Expand All @@ -41,30 +41,30 @@
# Weekdays on which the backup job should run. Defaults to `*`. This parameter is passed directly to the cron resource.
#
class postgresql::backup::pg_dump (
String[1] $dir,
Boolean $compress = true,
Array $databases = [],
Boolean $delete_before_dump = false,
String[1] $dir_group = '0',
String[1] $dir_mode = '0700',
String[1] $dir_owner = 'root',
Enum['present','absent'] $ensure = 'present',
Enum['plain','custom','directory','tar'] $format = 'plain',
Boolean $install_cron = true,
Boolean $manage_user = false,
Array $optional_args = [],
Stdlib::Absolutepath $pgpass_path = '/root/.pgpass',
Integer $rotate = 30,
Stdlib::Absolutepath $script_path = '/usr/local/sbin/pg_dump.sh',
Stdlib::Absolutepath $success_file_path = '/tmp/pgbackup_success',
String[1] $template = 'postgresql/pg_dump.sh.epp',
Array $time = ['23', '5'],
String[1] $weekday = '*',
Optional[Variant[String, Sensitive[String]]] $db_password = undef,
Optional[String[1]] $db_user = undef,
Optional[String[1]] $package_name = undef,
Optional[String[1]] $post_script = undef,
Optional[String[1]] $pre_script = undef,
String[1] $dir,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll note I hate alignment on data types since it creates so much noise in the git log. The old style was much better IMHO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understandable. However, we are currently favouring a more 'readable' and structured style.

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't help code readability much, but it makes diffs harder to read and review. +1 for not doing alignment.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd even argue it's less readable if you have a long type (like the version you wrote for the package ensure) there is a lot of whitespace, so it becomes hard to see which data types belong to which variables.

Copy link
Member

Choose a reason for hiding this comment

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

Personally I find it best for when I have to search out a specific variable in the list, especially for larger modules that have upwards of a dozen.
I've always found it easier to track down that one that I'm looking for.

Variant[Enum['present', 'absent', 'purged', 'disabled', 'installed', 'latest'], String[1]] $ensure = 'present',
Boolean $compress = true,
Array $databases = [],
Boolean $delete_before_dump = false,
String[1] $dir_group = '0',
String[1] $dir_mode = '0700',
String[1] $dir_owner = 'root',
Enum['plain','custom','directory','tar'] $format = 'plain',
Boolean $install_cron = true,
Boolean $manage_user = false,
Array $optional_args = [],
Stdlib::Absolutepath $pgpass_path = '/root/.pgpass',
Integer $rotate = 30,
Stdlib::Absolutepath $script_path = '/usr/local/sbin/pg_dump.sh',
Stdlib::Absolutepath $success_file_path = '/tmp/pgbackup_success',
String[1] $template = 'postgresql/pg_dump.sh.epp',
Array $time = ['23', '5'],
String[1] $weekday = '*',
Optional[Variant[String, Sensitive[String]]] $db_password = undef,
Optional[String[1]] $db_user = undef,
Optional[String[1]] $package_name = undef,
Optional[String[1]] $post_script = undef,
Optional[String[1]] $pre_script = undef,
) {
# Install required packages
if $package_name {
Expand Down
8 changes: 4 additions & 4 deletions manifests/client.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
# @param package_ensure
# Ensure the client package is installed
class postgresql::client (
Enum['file', 'absent'] $file_ensure = 'file',
Stdlib::Absolutepath $validcon_script_path = $postgresql::params::validcon_script_path,
String[1] $package_name = $postgresql::params::client_package_name,
String[1] $package_ensure = 'present'
Enum['file', 'absent'] $file_ensure = 'file',
Stdlib::Absolutepath $validcon_script_path = $postgresql::params::validcon_script_path,
String[1] $package_name = $postgresql::params::client_package_name,
Variant[Enum['present', 'absent', 'purged', 'disabled', 'installed', 'latest'], String[1]] $package_ensure = 'present',
) inherits postgresql::params {
if $package_name != 'UNSET' {
package { 'postgresql-client':
Expand Down
2 changes: 1 addition & 1 deletion manifests/dnfmodule.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#
# @api private
class postgresql::dnfmodule (
String[1] $ensure = 'installed',
Variant[Enum['present', 'absent', 'purged', 'disabled', 'installed', 'latest'], String[1]] $ensure = 'installed',
String[1] $module = 'postgresql',
) {
package { 'postgresql dnf module':
Expand Down
109 changes: 55 additions & 54 deletions manifests/globals.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
# Whether to manage the postgresql conf file permissions. This means owner,
# group and mode. Contents are not managed but should be managed through
# postgresql::server::config_entry.
# @param manage_selinux Allows Puppet to manage the appropriate configuration file for selinux.
#
# @param manage_datadir Set to false if you have file{ $datadir: } already defined
# @param manage_logdir Set to false if you have file{ $logdir: } already defined
Expand All @@ -95,72 +96,72 @@
#
#
class postgresql::globals (
$client_package_name = undef,
$server_package_name = undef,
$contrib_package_name = undef,
$devel_package_name = undef,
$java_package_name = undef,
$docs_package_name = undef,
$perl_package_name = undef,
$plperl_package_name = undef,
$plpython_package_name = undef,
$python_package_name = undef,
$postgis_package_name = undef,
Optional[String[1]] $client_package_name = undef,
Optional[String[1]] $server_package_name = undef,
Optional[String[1]] $contrib_package_name = undef,
Optional[String[1]] $devel_package_name = undef,
Optional[String[1]] $java_package_name = undef,
Optional[String[1]] $docs_package_name = undef,
Optional[String[1]] $perl_package_name = undef,
Optional[String[1]] $plperl_package_name = undef,
Optional[String[1]] $plpython_package_name = undef,
Optional[String[1]] $python_package_name = undef,
Optional[String[1]] $postgis_package_name = undef,

$service_name = undef,
$service_provider = undef,
$service_status = undef,
$default_database = undef,
Optional[String[1]] $service_name = undef,
Optional[String[1]] $service_provider = undef,
Optional[String[1]] $service_status = undef,
Optional[String[1]] $default_database = undef,

$validcon_script_path = undef,
Optional[String[1]] $validcon_script_path = undef,

$initdb_path = undef,
$createdb_path = undef,
$psql_path = undef,
$pg_hba_conf_path = undef,
$pg_ident_conf_path = undef,
$postgresql_conf_path = undef,
Optional[Stdlib::Filemode] $postgresql_conf_mode = undef,
$recovery_conf_path = undef,
$default_connect_settings = {},
Optional[Variant[String[1], Stdlib::Absolutepath]] $initdb_path = undef,
Optional[Variant[String[1], Stdlib::Absolutepath]] $createdb_path = undef,
Optional[Variant[String[1], Stdlib::Absolutepath]] $psql_path = undef,
Optional[Variant[String[1], Stdlib::Absolutepath]] $pg_hba_conf_path = undef,
Optional[Variant[String[1], Stdlib::Absolutepath]] $pg_ident_conf_path = undef,
Optional[Variant[String[1], Stdlib::Absolutepath]] $postgresql_conf_path = undef,
Optional[Stdlib::Filemode] $postgresql_conf_mode = undef,
Optional[Variant[String[1], Stdlib::Absolutepath]] $recovery_conf_path = undef,
Hash $default_connect_settings = {},

$pg_hba_conf_defaults = undef,
Optional[Boolean] $pg_hba_conf_defaults = undef,

$datadir = undef,
$confdir = undef,
$bindir = undef,
$xlogdir = undef,
$logdir = undef,
$log_line_prefix = undef,
$manage_datadir = undef,
$manage_logdir = undef,
$manage_xlogdir = undef,
Optional[String[1]] $datadir = undef,
Optional[String[1]] $confdir = undef,
Optional[String[1]] $bindir = undef,
Optional[String[1]] $xlogdir = undef,
Optional[String[1]] $logdir = undef,
Optional[String[1]] $log_line_prefix = undef,
Optional[Boolean] $manage_datadir = undef,
Optional[Boolean] $manage_logdir = undef,
Optional[Boolean] $manage_xlogdir = undef,

$user = undef,
$group = undef,
Optional[String[1]] $user = undef,
Optional[String[1]] $group = undef,

$version = undef,
$postgis_version = undef,
$repo_proxy = undef,
$repo_baseurl = undef,
$yum_repo_commonurl = undef,
Optional[String[1]] $version = undef,
Optional[String[1]] $postgis_version = undef,
Optional[String[1]] $repo_proxy = undef,
Optional[String[1]] $repo_baseurl = undef,
Optional[String[1]] $yum_repo_commonurl = undef,

$needs_initdb = undef,
Optional[Boolean] $needs_initdb = undef,

$encoding = undef,
$locale = undef,
$data_checksums = undef,
$timezone = undef,
Optional[String[1]] $encoding = undef,
Optional[String[1]] $locale = undef,
Optional[String[1]] $data_checksums = undef,
Optional[String[1]] $timezone = undef,

$manage_pg_hba_conf = undef,
$manage_pg_ident_conf = undef,
$manage_recovery_conf = undef,
$manage_postgresql_conf_perms = undef,
$manage_selinux = undef,
Optional[Boolean] $manage_pg_hba_conf = undef,
Optional[Boolean] $manage_pg_ident_conf = undef,
Optional[Boolean] $manage_recovery_conf = undef,
Optional[Boolean] $manage_postgresql_conf_perms = undef,
Optional[Boolean] $manage_selinux = undef,

$manage_package_repo = undef,
Optional[Boolean] $manage_package_repo = undef,
Boolean $manage_dnf_module = false,
$module_workdir = undef,
Optional[String[1]] $module_workdir = undef,
) {
# We are determining this here, because it is needed by the package repo
# class.
Expand Down
6 changes: 3 additions & 3 deletions manifests/lib/devel.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
#
#
class postgresql::lib::devel (
String $package_name = $postgresql::params::devel_package_name,
String[1] $package_ensure = 'present',
Boolean $link_pg_config = $postgresql::params::link_pg_config
Variant[Enum['present', 'absent', 'purged', 'disabled', 'installed', 'latest'], String[1]] $package_ensure = 'present',
String $package_name = $postgresql::params::devel_package_name,
Boolean $link_pg_config = $postgresql::params::link_pg_config,
) inherits postgresql::params {
if $facts['os']['family'] == 'Gentoo' {
fail('osfamily Gentoo does not have a separate "devel" package, postgresql::lib::devel is not supported')
Expand Down
4 changes: 2 additions & 2 deletions manifests/lib/docs.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
#
#
class postgresql::lib::docs (
String $package_name = $postgresql::params::docs_package_name,
String[1] $package_ensure = 'present',
String $package_name = $postgresql::params::docs_package_name,
Variant[Enum['present', 'absent', 'purged', 'disabled', 'installed', 'latest'], String[1]] $package_ensure = 'present',
) inherits postgresql::params {
package { 'postgresql-docs':
ensure => $package_ensure,
Expand Down
4 changes: 2 additions & 2 deletions manifests/lib/java.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
# Specifies whether the package is present.
#
class postgresql::lib::java (
String $package_name = $postgresql::params::java_package_name,
String[1] $package_ensure = 'present'
String $package_name = $postgresql::params::java_package_name,
Variant[Enum['present', 'absent', 'purged', 'disabled', 'installed', 'latest'], String[1]] $package_ensure = 'present',
) inherits postgresql::params {
package { 'postgresql-jdbc':
ensure => $package_ensure,
Expand Down
4 changes: 2 additions & 2 deletions manifests/lib/perl.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
# Ensure the perl libs for postgresql are installed.
#
class postgresql::lib::perl (
String $package_name = $postgresql::params::perl_package_name,
String[1] $package_ensure = 'present'
String $package_name = $postgresql::params::perl_package_name,
Variant[Enum['present', 'absent', 'purged', 'disabled', 'installed', 'latest'], String[1]] $package_ensure = 'present',
) inherits postgresql::params {
package { 'perl-DBD-Pg':
ensure => $package_ensure,
Expand Down
4 changes: 2 additions & 2 deletions manifests/lib/python.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
# Ensure the python libs for postgresql are installed.
#
class postgresql::lib::python (
String[1] $package_name = $postgresql::params::python_package_name,
String[1] $package_ensure = 'present'
String[1] $package_name = $postgresql::params::python_package_name,
Variant[Enum['present', 'absent', 'purged', 'disabled', 'installed', 'latest'], String[1]] $package_ensure = 'present',
) inherits postgresql::params {
package { 'python-psycopg2':
ensure => $package_ensure,
Expand Down
8 changes: 4 additions & 4 deletions manifests/repo.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# @api private
class postgresql::repo (
$version = undef,
$proxy = undef,
$baseurl = undef,
$commonurl = undef,
Optional[String[1]] $version = undef,
Optional[String[1]] $proxy = undef,
Optional[String[1]] $baseurl = undef,
Optional[String[1]] $commonurl = undef,
) {
case $facts['os']['family'] {
'RedHat', 'Linux': {
Expand Down
Loading