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

initial API proposal for manager #42

Merged
merged 9 commits into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 45 additions & 11 deletions service/lib/dinstaller/dbus/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
# find current contact information at www.suse.com.

require "dbus"
require "dinstaller/manager"

module DInstaller
module DBus
Expand All @@ -33,29 +34,62 @@ class Manager < ::DBus::Object
MANAGER_INTERFACE = "org.opensuse.DInstaler.Manager1"
private_constant :MANAGER_INTERFACE

attr_reader :installer, :logger
attr_reader :logger

# @param installer [Yast2::Installer] YaST installer instance
# @param args [Array<Object>] ::DBus::Object arguments
def initialize(installer, logger)
@installer = installer
def initialize(logger)
@logger = logger

# @available_base_products = installer.products
@backend = ::DInstaller::Manager.instance
@backend.progress.add_on_change_callback do
PropertiesChanged(MANAGER_INTERFACE, {}, ["Progress"])
end
@backend.add_status_callback do
PropertiesChanged(MANAGER_INTERFACE, { "Status" => @backend.status.id }, [])
end

super(PATH)
end

dbus_interface MANAGER_INTERFACE do
dbus_method :probe, "out result:u" do
# TODO
0
dbus_method :probe, "" do
@backend.probe
end

dbus_method :commit, "out result:u" do
# TODO
0
dbus_method :commit, "" do
@backend.commit
end

# Enum list for statuses. Possible values:
# 0 : error ( it can be read from progress message )
# 1 : probing
# 2 : probed
# 3 : installing
# 4 : installed
dbus_reader :status, "u"

# Progress has struct with values:
# s message
# t total major steps to do
# t current major step (0-based)
# t total minor steps. Can be zero which means no minor steps
# t current minor step
dbus_reader :progress, "(stttt)"
Copy link
Contributor

Choose a reason for hiding this comment

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

So a D-Bus client can poll the progress by reading the property, or listen for PropertiesChanged signals, right?

InstallationProgress has @dbus_obj&.Progress(...args...). How is that related? Which class (interface) is @dbus_obj?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly. I see that pattern often.
InstallationProgress will be dropped. Plan is to use Progress.new in dbus Manager object and assign callback to it that raise signal. And attach progress to data structure read from that progress.object.

end

def status
@backend.status.id
end

def progress
prg = @backend.progress
[
prg.message,
prg.total_steps,
prg.current_step,
prg.total_minor_steps,
prg.current_minor_step
].freeze
end
end
end
Expand Down
8 changes: 5 additions & 3 deletions service/lib/dinstaller/dbus/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
require "dinstaller/dbus/language"
require "dinstaller/dbus/software"
require "dinstaller/dbus/users"
require "dinstaller/installer"
require "dinstaller/manager"

module DInstaller
module DBus
Expand Down Expand Up @@ -73,7 +73,7 @@ def dbus_objects
end

def manager_bus
@manager_bus ||= DInstaller::DBus::Manager.new(installer, @logger)
@manager_bus ||= DInstaller::DBus::Manager.new(@logger)
end

def language_dbus
Expand All @@ -89,7 +89,9 @@ def users_dbus
end

def installer
@installer ||= DInstaller::Installer.new(logger: @logger).tap(&:probe)
# TODO: this god object should not be needed anymore when all dbus API is adapted
# just that probe should be kept to get installer probed ASAP
@installer ||= DInstaller::Manager.instance.tap(&:probe)
end
end
end
Expand Down
92 changes: 0 additions & 92 deletions service/lib/dinstaller/installation_progress.rb

This file was deleted.

27 changes: 7 additions & 20 deletions service/lib/dinstaller/installer_status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,16 @@
module DInstaller
# This class represents the installer status
class InstallerStatus
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe having a class is too much. Is this expected to grow?

Copy link
Contributor

Choose a reason for hiding this comment

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

Idea: What about having something like DInstaller::StatusManager class, which defines possible statuses, implement logic for changing status, for calling callbacks and so on? And DInstaller::Manager would simply instanciate a StatusManager and would request a status change when needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I am not sure. I try to not touch this part :) Just change it from string to unsigned.

class << self
# Returns all the possible installer statuses
#
# @return [Array<InstallerStatus>] Installer status
def all
@all ||= constants
.map { |c| InstallerStatus.const_get(c) }
.select { |c| c.is_a?(InstallerStatus) }
end
end

attr_reader :id, :name
attr_reader :id

def initialize(id, name)
def initialize(id)
@id = id
@name = name
end

IDLE = new(0, "Idle")
PROBING = new(1, "Probing")
PARTITIONING = new(2, "Partitioning")
INSTALLING = new(3, "Installing")
FINISHED = new(4, "Finished")
ERROR = new(5, "Error")
ERROR = new(0)
PROBING = new(1)
PROBED = new(2)
INSTALLING = new(3)
INSTALLED = new(4)
end
end
Loading