Skip to content
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

(MODULES-10893) Fix Last Day Of Month Trigger #175

Merged

Conversation

RandomNoun7
Copy link
Contributor

Prior to this change, attempting to create a trigger that will run on
the last day of each selected month will fail. The error claims that a
a value is out of range

OLE error code:0 in <Unknown>
      <No Description>
    HRESULT error code:0x8002000a
      Out of present range.

Microsoft documentation claims in one place that the correct way to
specify the last day of a month is to effectively translate a bitmask
value of day 32 and pass that into the {get:set}_DaysOfMonth method
on the trigger object. The module was previously coded to do exactly
this. The provider and it's helpers would catch the string value 'last'
in the on property of a monthly trigger and translate it into the
bit mask. This is what resulted in the error, because it turns out that
the position for day 32 in the bit mast is outside the acceptable range
of values for this property.

This PR changes strategy and instead relies on a different section
of the documentation in which an entirely different property on the
trigger object called RunOnLastDayOfMonth is set.

The code now catches the string value 'last' in the on property of
the trigger, and sets this boolean on the trigger object.

Testing has shown this to be an effective method of managing this
property of a trigger.

@RandomNoun7 RandomNoun7 requested a review from a team as a code owner December 16, 2020 16:15
@RandomNoun7 RandomNoun7 force-pushed the MODULES-10893-fix-last-day-of-month branch from 5f24243 to a59945d Compare December 16, 2020 17:53
Copy link
Contributor

@sanfrancrisko sanfrancrisko left a comment

Choose a reason for hiding this comment

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

Just one minor issue, but overall looks good 👍

Thanks for picking this up and addressing - took some work off my plate today 😃

@RandomNoun7 RandomNoun7 force-pushed the MODULES-10893-fix-last-day-of-month branch from a59945d to f870e90 Compare December 16, 2020 18:41
@RandomNoun7 RandomNoun7 marked this pull request as draft December 16, 2020 18:42
@RandomNoun7 RandomNoun7 force-pushed the MODULES-10893-fix-last-day-of-month branch 4 times, most recently from d1538e0 to 7039ecf Compare December 16, 2020 22:07
@codecov-io
Copy link

codecov-io commented Dec 16, 2020

Codecov Report

Merging #175 (779908b) into main (106d092) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #175      +/-   ##
==========================================
+ Coverage   94.97%   95.02%   +0.04%     
==========================================
  Files           6        6              
  Lines         836      844       +8     
==========================================
+ Hits          794      802       +8     
  Misses         42       42              
Impacted Files Coverage Δ
lib/puppet/type/scheduled_task.rb 92.85% <ø> (ø)
lib/puppet_x/puppetlabs/scheduled_task/trigger.rb 94.90% <100.00%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 106d092...779908b. Read the comment docs.

@RandomNoun7 RandomNoun7 force-pushed the MODULES-10893-fix-last-day-of-month branch 3 times, most recently from 682314c to 5995d0e Compare December 16, 2020 22:49
@RandomNoun7 RandomNoun7 marked this pull request as ready for review December 16, 2020 22:56
@RandomNoun7 RandomNoun7 force-pushed the MODULES-10893-fix-last-day-of-month branch 2 times, most recently from 1ec8c87 to 1133c06 Compare December 16, 2020 23:14
Prior to this change, attempting to create a trigger that will run on
the last day of each selected month will fail. The error claims that a
a value is out of range

```
OLE error code:0 in <Unknown>
      <No Description>
    HRESULT error code:0x8002000a
      Out of present range.
```

Microsoft [documentation] claims in one place that the correct way to
specify the last day of a month is to effectively translate a bitmask
value of day 32 and pass that into the `{get:set}_DaysOfMonth` method
on the trigger object. The module was previously coded to do exactly
this. The provider and it's helpers would catch the string value 'last'
in the `on` property of a `monthly` trigger and translate it into the
bit mask. This is what resulted in the error, because it turns out that
the position for day 32 in the bit mast is outside the acceptable range
of values for this property.

This PR changes strategy and instead relies on a different section
of the documentation in which an entirely different property on the
trigger object called [RunOnLastDayOfMonth] is set.

The code now catches the string value 'last' in the `on` property of
the trigger, and sets this boolean on the trigger object.

Testing has shown this to be an effective method of managing this
property of a trigger.

[documentation]: https://docs.microsoft.com/en-gb/windows/win32/api/taskschd/nf-taskschd-imonthlytrigger-get_daysofmonth
[RunOnLastDayOfMonth]: https://docs.microsoft.com/en-gb/windows/win32/api/taskschd/nf-taskschd-imonthlytrigger-put_runonlastdayofmonth
@RandomNoun7 RandomNoun7 force-pushed the MODULES-10893-fix-last-day-of-month branch from 1133c06 to 779908b Compare December 16, 2020 23:16
Copy link
Contributor

@sanfrancrisko sanfrancrisko left a comment

Choose a reason for hiding this comment

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

Excellent, thanks for this @RandomNoun7 👍

@sanfrancrisko sanfrancrisko merged commit 46bf890 into puppetlabs:main Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants