rose icon indicating copy to clipboard operation
rose copied to clipboard

attempt to handle rose rsync can_pull errors better

Open wxtim opened this issue 7 months ago • 2 comments

Fixes problem not recorded in an issue:

Issue

Rose has a routine to work out which file_loc_handler should be used based on the content of the source= field. However, the function called by the rsync loc handler will attempt to run

# loc handler = foo:bar
ssh -n foo test -e bar

As part of the method used to decide if the source is an Rsync handler. This fails in a difficult to debug way, as it's unclear whether the rsync check routine failed because the host was unreachable, or the file didn't exist, or the source was not meant to be a location reached by rsync, or was just garbage.

This PR

  • Ensures that Rsync is always the last handler tried.
  • Allows the rsync handler to raise more informative errors.
  • Give the user more useful detail if the loc handler cannot be identified because this ssh check fails.

wxtim avatar Apr 24 '25 15:04 wxtim

Did you manage to work out why the command stderr was not logged?

oliver-sanders avatar Apr 25 '25 10:04 oliver-sanders

Did you manage to work out why the command stderr was not logged?

Because no one made Rose log it.

When I was testing it in front of you and there was no output, that was because test -e nonexistant doesn't return any stdout/err.

On consideration, I should probably have a specific error message for this outcome!

wxtim avatar Apr 25 '25 14:04 wxtim

Because no one made Rose log it.

I would expect there to be some standard way of doing this (else we would have to do this for every single command individually).

From a quick code skim, it looks like this is the centralised logic:

https://github.com/metomi/rose/blob/26a6066e9f8d53caa89b7d976a0310e355e52187/metomi/rose/popen.py#L44-L61

So, I think the issue here, is that this command swallows the RosePopenError rather than allowing it to bubble up and be reported as intended?

oliver-sanders avatar Apr 28 '25 11:04 oliver-sanders

@oliver-sanders Poke

wxtim avatar Oct 16 '25 11:10 wxtim

You may want to consider changing to a bugfix branch.

Plz squash merge.

oliver-sanders avatar Oct 23 '25 11:10 oliver-sanders