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.

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

Save the chronyd service according to user preference #116

Merged
merged 4 commits into from Sep 10, 2018

Conversation

Projects
None yet
4 participants
@dgdavid
Member

dgdavid commented Sep 7, 2018

Related to https://bugzilla.suse.com/show_bug.cgi?id=1075039


Issue

Despite user unchecks "Run NTP as daemon", the chronyd service keeps active and enabled.

Why?

It seems that the timezone dialog (yast-country) is missing the run_service param at the time to call the NtpClientProposalClient#Write (yast-ntp-client). See yast-country/timezone/src/include/timezone/dialogs.rb:L980.

In consequence, since the default value for that param is true, the service is being started and enabled always, without matter what the user has chosen.

Implemented solution

Since NtpClient.run_service has stored the correct value according to the user preferences, this value is now used as default value for run_service in the NtpClientProposalClient#Write.

@dgdavid dgdavid requested a review from jreidinger Sep 7, 2018

dgdavid added some commits Sep 7, 2018

Fix default value for `run_service` in NtpClientProposalClient#Write
When the `run_service` param is missing, the default value should be the
stored in NtpClient.run_service instead `true` avoiding to ignored the
user preference (i.e., when the `Run NTP as daemon` is unchecked in the
yast-country Timezone dialog (Other settings)).

Related to bsc#1075039
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 7, 2018

Coverage Status

Coverage increased (+13.9%) to 53.228% when pulling 132d8de on feature/fix-bsc-1075039 into 6d99c3d on master.

coveralls commented Sep 7, 2018

Coverage Status

Coverage increased (+13.9%) to 53.228% when pulling 132d8de on feature/fix-bsc-1075039 into 6d99c3d on master.

@@ -0,0 +1,169 @@
require_relative "test_helper"
require_relative "../src/clients/ntp-client_proposal.rb"

This comment has been minimized.

@jreidinger

jreidinger Sep 7, 2018

Member

this is a bit problematic, as it execute client program. In this case it just write error to log, but it can change in future ( e.g. many client raise exception if parameter missing or start UI). That is reason why code in src/clients have also zero code coverage.

@jreidinger

jreidinger Sep 7, 2018

Member

this is a bit problematic, as it execute client program. In this case it just write error to log, but it can change in future ( e.g. many client raise exception if parameter missing or start UI). That is reason why code in src/clients have also zero code coverage.

This comment has been minimized.

@dgdavid

dgdavid Sep 10, 2018

Member

Ops!

Sorry, I forgot that. Probably you already told me in a previous PR.

So, shall I to discard this test? Or mock the call to log.error in this case and keep the test?

What do you prefer or think is better?

Thank you.

@dgdavid

dgdavid Sep 10, 2018

Member

Ops!

Sorry, I forgot that. Probably you already told me in a previous PR.

So, shall I to discard this test? Or mock the call to log.error in this case and keep the test?

What do you prefer or think is better?

Thank you.

This comment has been minimized.

@jreidinger

jreidinger Sep 10, 2018

Member

keep it as it is for now

@jreidinger

jreidinger Sep 10, 2018

Member

keep it as it is for now

@dgdavid dgdavid merged commit 6ef7128 into master Sep 10, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+13.9%) to 53.228%
Details

@dgdavid dgdavid deleted the feature/fix-bsc-1075039 branch Sep 10, 2018

@lslezak

This comment has been minimized.

Show comment
Hide comment
@lslezak

lslezak Sep 10, 2018

Member

✔️ Public Jenkins job #5 successfully finished
✔️ Created OBS submit request #634715

Member

lslezak commented Sep 10, 2018

✔️ Public Jenkins job #5 successfully finished
✔️ Created OBS submit request #634715

@lslezak

This comment has been minimized.

Show comment
Hide comment
@lslezak

lslezak Sep 10, 2018

Member

✔️ Internal Jenkins job #5 successfully finished
✔️ Created IBS submit request #171738

Member

lslezak commented Sep 10, 2018

✔️ Internal Jenkins job #5 successfully finished
✔️ Created IBS submit request #171738

@dgdavid dgdavid referenced this pull request Sep 10, 2018

Closed

Squad Sprint 62 Post Draft #163

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment