stefangweichinger / ansible-rclone

ansible role for rclone :: https://galaxy.ansible.com/stefangweichinger/ansible_rclone
160 stars 57 forks source link

Register variable for rclone config setup task result #133

Closed tigattack closed 1 year ago

tigattack commented 1 year ago

This will enable users to act on the result of the rclone config setup task when including the role in a playbook, role, etc.

For example:

- name: Install and configure rclone
  ansible.builtin.include_role:
    name: stefangweichinger.ansible_rclone
  vars:
    rclone_configs:
      - name: ExampleGoogleDriveRemote
        properties:
          type: drive
          client_id: 12345
          client_secret: 67890

- name: Restart rclone
  ansible.builtin.systemd:
    name: rclone.service
    state: restarted
  when: setup_rclone_config.changed
stefangweichinger commented 1 year ago

Thanks for the PR. Busy right now, it will take me a bit to put your example into README.md etc

tigattack commented 1 year ago

Hey @stefangweichinger, I've added some readme info, let me know if you have any further suggestions.

stefangweichinger commented 1 year ago

@tigattack ah, you understood my hint ;-) Great work, looks very good at a first look. I might merge this later today. Thanks a lot for your contribution.

tigattack commented 1 year ago

No problem. This role's been super helpful for me, so thank you for your work!

stefangweichinger commented 1 year ago

looks great. I read through your lines in README.md: do you think it would be a good feature to also deploy rclone.service? This might be a larger todo: different target distros need different users, paths etc ... just thinking.

stefangweichinger commented 1 year ago

merged your PR, I had issues with tagging ... still not used to that versioning stuff. the published galaxy role should be tagged 0.1.2 after the release pipeline is through. let me know if things worked. thanks

tigattack commented 1 year ago

Thanks for the merge, I'll let you know if there's any problems.

I like the idea of optionally deploying a systemd unit, but as you say, this would be a larger task. I'll look into it when I get a chance as I'm currently doing this in separate tasks, so it would be useful for me if it could be combined.

stefangweichinger commented 1 year ago

I think it would be beyond the scope of this role when I look at examples for a systemd unit for rclone. But I am open to discussion, sure.

tigattack commented 1 year ago

I was thinking it over further just as your comment came in and pretty much came to the same conclusion.

It could certainly be done, but a choice would have to be made on what sort of balance to strike between complexity and maintainability; Rclone systemd units can be very simple, but quickly grow in complexity with small feature changes.

Given how many features rclone supports, the use cases and configuration of a service can vary wildly. If a single option for the service were to be omitted from this role, it would immediately render the feature useless for a given use case.

It's probably not worth the effort it would take to maintain and keep it useful for everyone, as nice as it would be in theory.

Edit: 0.1.2 working great btw 👍

stefangweichinger commented 1 year ago

I agree with you: too complex. It might be heplful to add some explanations to the README like "what the role does, what the role does not" where we could explain the reasons. Just to enhance the overall quality. But that's a nice-to-have, no must.

Great that 0.1.2 works for you, thanks for the PR once more, looking forward to any more ideas.