Skip to content

added possibility to specify rsync binary using option rsync_binary #20

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

captainhunt
Copy link

Useful if you need to fix rsync to a specific version and dont want to require each users to modify their path.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@@ -38,7 +40,7 @@ def get_rsync_command(source: str,
if options is None:
options = []

return ['rsync',
return [rsync_cmd,
Copy link
Owner

Choose a reason for hiding this comment

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

the argument is called rsync_binary, but here, it is rsync_cmd

Comment on lines +11 to +15
rsync_args = inspect.getfullargspec(get_rsync_command)[0]
subp_kwargs = {}
for k, v in kwargs.items():
if k not in rsync_args:
subp_kwargs[k] = v
Copy link
Owner

@gchamon gchamon May 29, 2022

Choose a reason for hiding this comment

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

maybe to avoid risking get_rsync_command and subprocess.run arguments collision, we could add a separate argument for sysrsync.run to pass those args to subprocess.run?

@gchamon gchamon mentioned this pull request May 29, 2022
@gchamon
Copy link
Owner

gchamon commented Dec 2, 2024

Sorry again. It's been a while (for some PRs quite longer than that), but I plan to give this project some attention (finally). Expect some updates this month.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants