Skip to content

Conversation

@glennsarti
Copy link
Contributor

TODO

Will supercede #6198

@puppetcla
Copy link

CLA signed by all contributors.

@glennsarti glennsarti force-pushed the pup-7894-tasksched2-attempt2 branch from dbfa648 to 1158718 Compare November 22, 2017 17:47
This commit adds a new class which has the same interface as the older V1
Scheduled Tasks (puppet/util/windows/taskscheduler.rb).  This facade class will
implement the V1 methods but using the V2 API calls.  This commit only adds the
minimum required implementations, and the rest with NotImplementedErrors.  These
will be populated in later commits.
This commit adds a non-default provider for the Scheduled Task type but instead
uses the V2 facade class.  This provider is in essence a verbatim copy of the
original provider (win32_taskscheduler) but changes the helper class to the
V2 API facade object.  This maintains backwards compatilbity and will allow the
older V1 implementation to be deprecated and then removed easily.
This commit changes the unit tests for the scheduled_task provider by running
the tests against both providers (win32_taskscheduler and
taskscheduler_api2). This ensures that any behaviors are consistent in both
providers.

The concrete class which services each provider is refactored into the
concrete_klass variable which is then used in the tests themselves.
@glennsarti glennsarti force-pushed the pup-7894-tasksched2-attempt2 branch from 1158718 to aeacc2f Compare November 22, 2017 18:04

class Puppet::Util::Windows::TaskScheduler2V1Task
# The error class raised if any task scheduler specific calls fail.
class Error < Puppet::Util::Windows::Error; end
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there should be a require 'puppet/util/windows' here so that this is unnecessary...

@task = nil

@tasksched.set_compatibility(@definition, Puppet::Util::Windows::TaskScheduler2::TASK_COMPATIBILITY_V1)
#@definition.compatibility = Puppet::Util::Windows::TaskScheduler2::TASK_COMPATIBILITY_V1
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment


# Delete the specified task name.
#
def delete(task)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we determined delete should be used somewhere


# Execute the current task.
#
def run
Copy link
Contributor

Choose a reason for hiding this comment

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

There are no callers for run anywhere, so not sure why it's here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's in the original class and this is binary compatible.

@glennsarti glennsarti force-pushed the pup-7894-tasksched2-attempt2 branch from aeacc2f to 5635e3a Compare November 22, 2017 22:16
Previously Puppet used the V1 API for scheduled tasks however this has been
deprecated in favor of the V2 Win32OLE objects.  This commit adds a module which
can query and modify tasks with the V2 API.  This module only implements the
required methods for the existing Scheduled Task providers, for example it does
implement methods to run, terminate tasks or get last run information or exit
codes.
Previously a basic implementation of the V2 facade class was introduced.  This
commit actually implements the required methods for the Scheduled Task
providers.  Note that many methods still raise Not Implemented errors as these
are not required by the Scheduled Task providers to manage state.
Previously the Scheduled Task API helpers did not have any tests to confirm
their behaviour.  This commit adds some basic tests to the Scheduled Task V2
module to ensure it can list, create, delete, and modify tasks on a Windows
system.  This commit also adds integration tests for the V2 facade class to
confirm that tasks created in the V1 API appear in the V2 facade and vice versa.
@glennsarti glennsarti force-pushed the pup-7894-tasksched2-attempt2 branch from 5635e3a to 6b87f37 Compare November 22, 2017 22:54
# https://msdn.microsoft.com/en-us/library/windows/desktop/aa383600(v=vs.85).aspx

# @api private
module Puppet::Util::Windows::TaskScheduler2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to split this into component module statements as per @Iristyle recommendation

Copy link
Contributor

@Iristyle Iristyle Nov 29, 2017

Choose a reason for hiding this comment

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

I thought @DLuCJ ran into requiring things to be split, but the PR that got merged didn't have the changes I was referring to (https://github.com/puppetlabs/puppet/pull/5114/files and https://github.com/puppetlabs/puppet/pull/5098/files) - so I seem to be misremembering there.

The reason to do this is based on Ruby scoping rules - see https://github.com/bbatsov/ruby-style-guide#namespace-definition for the surprising behavior!

@Iristyle
Copy link
Contributor

Iristyle commented Jan 3, 2018

Leaving open for the moment, but this should close shortly for work to proceed in the new https://github.com/puppetlabs/puppetlabs-scheduled_task repo

@Iristyle
Copy link
Contributor

This has shipped in a different form in puppetlabs/puppetlabs-scheduled_task#6

@Iristyle Iristyle closed this Jan 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants