Rdb save incremental fsync by thejambavan · Pull Request #495 · voxpupuli/puppet-redis

thejambavan · 2023-11-09T10:46:40Z

Pull Request (PR) description

minor change to the redis.conf EPP template to make it possible to disable rdb_save_incremental_fsync

This Pull Request (PR) fixes the following issues

Fixes #494

kenyon

I don't see how this changes anything. If you are providing a non-undef value to redis::rdb_save_incremental_fsync, then it will render either rdb-save-incremental-fsync yes or rdb-save-incremental-fsync no.

To be sure, you could add a test like

puppet-redis/spec/classes/redis_spec.rb

Lines 1683 to 1691 in e8d71af

    
           describe 'test rdb-save-incremental-fsync for redis6' do 
        
             let(:params) do 
        
               { 
        
                 rdb_save_incremental_fsync: true, 
        
               } 
        
             end 
        
             it { is_expected.to contain_file(config_file_orig).with('content' => %r{^rdb-save-incremental-fsync yes$}) } 
        
           end

that sets rdb_save_incremental_fsync to false and checks that the config files contains rdb-save-incremental-fsync no.

kenyon

Actually I see the problem now. The if $rdb_save_incremental_fsync evaluates to false when you set rdb_save_incremental_fsync to false, so the line doesn't get rendered. But your patch doesn't fix that bug.

thejambavan · 2023-11-10T10:23:15Z

I have corrected the if to an unless Undef and have added tests which seem to do the job (apologies for multiple formatting commits) for all three states:

Undef: no config file entry, aka "the default" which may have been "no" in the past but is now "yes"
true: yes
false: no

I have separately verified that this works as intended on a real-world instance.

edit: i clearly do not now how to use rspec properly

kenyon · 2023-11-10T20:39:09Z

spec/classes/redis_spec.rb

+      describe 'test rdb-save-incremental-fsync Undef for redis6' do
+        let(:params) do
+          {
+            rdb_save_incremental_fsync: Undef,


Suggested change

rdb_save_incremental_fsync: Undef,

rdb_save_incremental_fsync: nil,

…ar with its behaviour

… hand 🤷

thejambavan · 2023-11-16T21:36:12Z

no matter what i do with the test, when i set:

            rdb_save_incremental_fsync: nil,

as part of the rspec test, i can't get the test to pass in the Puppet/ruby stages. I don't see any other tests for undef/nil, and have not had any luck finding good examples elsewhere.

thejambavan added 3 commits November 8, 2023 16:06

make it possible to turn off rdb-save-incremental-fsync

c261392

make it possible to turn off rdb-save-incremental-fsync

28af3ce

wrap in an if to account for undef

2d4375c

kenyon reviewed Nov 9, 2023

View reviewed changes

kenyon requested changes Nov 9, 2023

View reviewed changes

kenyon added needs-tests needs-work not ready to merge just yet labels Nov 9, 2023

thejambavan added 6 commits November 10, 2023 08:34

comparison to Undef and test

52e777e

comparison to Undef and test (fix typo)

75f2e7c

rspec formatting

8453017

rspec formatting

88179d2

rspec test for Undef

d59524b

rspec formatting

a6db2e7

kenyon reviewed Nov 10, 2023

View reviewed changes

thejambavan added 15 commits November 16, 2023 15:39

minot commit bump to trigger CI

98b7504

Undef -> nil in spec

c4e308e

Merge branch 'master' into rdb-save-incremental-fsync

5cbb8fe

attempt test for negation using regex rather than as i am less famili…

757e9d9

…ar with its behaviour

perhaps i want instead

440a70c

maybe i made an infocorrect assumption in the EPP template

5e2f74b

sometimes it can be a string

c5b865a

reorder tests to test undef/nil variant first

442dfdc

reorder tests to test undef/nil variant first

01d5160

reorder tests to test undef/nil variant first

976ad0a

misspelled as rdb_save_incremental_fsync

52fe4a7

easier to test for a commented-out line

2fe204b

easier to test for a commented-out line

cd91071

easier to test for a commented-out line

0fa6900

i give up on this test. It works in reality, I could really do with a…

00a1d33

… hand 🤷

thejambavan added 4 commits November 16, 2023 20:34

i give up on this test. It works in reality, I could really do with a…

64a690c

… hand 🤷

last idea: undef-as-string rodjek/rspec-puppet#279

a7f18d2

last idea: undef-as-string rodjek/rspec-puppet#279

8a6d21e

i give up on this test. It works in reality, I could really do with a…

d101011

… hand 🤷

bump to trigger tests

58af36c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rdb save incremental fsync#495

Rdb save incremental fsync#495
thejambavan wants to merge 29 commits intovoxpupuli:masterfrom
thejambavan:rdb-save-incremental-fsync

thejambavan commented Nov 9, 2023

Uh oh!

kenyon left a comment

Uh oh!

kenyon left a comment •

edited

Loading

Uh oh!

thejambavan commented Nov 10, 2023 •

edited

Loading

Uh oh!

kenyon Nov 10, 2023

Uh oh!

thejambavan commented Nov 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

	describe 'test rdb-save-incremental-fsync for redis6' do
	let(:params) do
	{
	rdb_save_incremental_fsync: true,
	}
	end

	it { is_expected.to contain_file(config_file_orig).with('content' => %r{^rdb-save-incremental-fsync yes$}) }
	end

	rdb_save_incremental_fsync: Undef,
	rdb_save_incremental_fsync: nil,

Conversation

thejambavan commented Nov 9, 2023

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Uh oh!

kenyon left a comment

Choose a reason for hiding this comment

Uh oh!

kenyon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thejambavan commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kenyon Nov 10, 2023

Choose a reason for hiding this comment

Uh oh!

thejambavan commented Nov 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

kenyon left a comment •

edited

Loading

thejambavan commented Nov 10, 2023 •

edited

Loading