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

RFC: Move Manager to Rust #857

Closed
wants to merge 1 commit into from
Closed

RFC: Move Manager to Rust #857

wants to merge 1 commit into from

Conversation

joseivanlopez
Copy link
Contributor

@joseivanlopez joseivanlopez commented Nov 10, 2023

Introduction

This branch starts the migration of the Manager service from Ruby to Rust code. For more details, see the associated HackWeek project https://hackweek.opensuse.org/23/projects/port-agamas-manager-to-rust.

Manager and progress reporting

Manager and other services implement the org.opensuse.Agama1.Progress D-Bus interface. That interface is used to report the progress of the current action being performed by the service.

That progress interface only has read-only properties:

org.opensuse.Agama1.Progress

NAME                         TYPE      SIGNATURE RESULT/VALUE FLAGS
.CurrentStep                 property  (ys)      0 ""         emits-change
.TotalSteps                  property  y         0            emits-change

And the code reporting progress should be able to start, step and finish a progress. For example:

progress.start(total_steps: 3)
progress.step("Reading data")
...
progress.step("Configuring")
...
progress.step("Writing data")
...
progress.finish

The calls to #start, #step or #finish changes the data of the progess (e.g., total steps and current step), so a PropertiesChanged signal should be properly emited in D-Bus in order to notify about changes in CurrentStep and TotalSteps properties.

Typically, D-Bus libraries like ruby-dbus or zbus automatically emit a signal when a property is set by means of a D-Bus property setter. But, the progress interface lacks of such setters (it only has read-only properties). Therefore, the progress should implement any kind of strategy for emiting D-Bus signasl as part of the #start, #step and #finish methods.

In short, the problem to solve consists on emiting a D-Bus signal from outside of the D-Bus interface.

Ruby solution in Agama

In the ruby code this problem was already solved by using callbacks:

class Progress
  def start(total_steps)
    @total_steps = total_steps
    @on_change_callbacks.each(&:run)
  end

  def on_change(&block)
    @on_change_callbacks ||= []
    @on_change_callbacks << block
  end
end

module DBus
  class Manager < DBus::Object
    def initialize(progress)
       progress.on_change do
          dbus_properties_changed(PROGRESS_INTERFACE, progress_properties, [])
       end
    end
  end
end

Basically, the D-Bus object adds a callback to Progress in order to emit a signal when progress changes.

RFC: Rust solution

This PR proposes a possible solution in Rust. In Agama we are using zbus crate for working with D-Bus. Again, emiting a property changed signal is trivial when you do it from the D-Bus interface implementation:

#[dbus_interface(name = "org.opensuse.Agama1.Progress")]
impl Progress {
    #[dbus_interface(property)]
    fn total_steps(&self) -> u8 {
        self.total_steps
    }
    async fn start(
        &mut self,
        total_steps: u8,
        #[zbus(signal_context)]
        ctxt: SignalContext<'_>,
    ) -> fdo::Result<()> {
        self.total_steps = total_steps;
        self.total_steps_changed(&ctxt).await?;
       
        Ok(())
    }
}

Note that for emiting a signal (e.g., #total_steps_changed) we need the signal context reference, which is automatically provided by zbus as parameter of the methods defining the D-Bus interface. But we don't want #start to be part of the D-Bus interface, so how could we get the signal context reference from outside the D-Bus interface?

The solution proposed by this PR consists on recovering and storing a reference to the D-Bus interface. Having the interface reference we can get the signal context and emit signals:

pub async fn export_interface(
    connection: &Connection,
    path: &str,
) -> Result<InterfaceRef<Progress>, Box<dyn std::error::Error>> {
    let progress = Progress::new();
    connection.object_server().at(path, progress).await?;

    let iface_ref = connection.object_server().interface::<_, Progress>(path).await?;
    let mut iface = iface_ref.get_mut().await;
    iface.set_iface_ref(iface_ref.clone());

    Ok(iface_ref.clone())
}

impl Progress {
    pub async fn start(&mut self, total_steps: u8) -> Resutl<(), Error> {
        self.total_steps = total_steps;
        self.total_steps_changed(self.iface_ref.signal_context()).await?;

        Ok(())
    }
}

#[dbus_interface(name = "org.opensuse.Agama1.Progress")]
impl Progress {
    #[dbus_interface(property)]
    fn total_steps(&self) -> u8 {
        self.total_steps
    }
}

Now #start does not belong to the D-Bus interface implementation. Services can call to Progress#start, which automatically emits the proper D-Bus signals.

@imobachgs
Copy link
Contributor

Thanks for the PoC! As we plan to move to an HTTP-based API, I would close this PR. Please, feel free to disagree 😉.

@imobachgs imobachgs closed this Feb 26, 2024
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.

2 participants