jenkinsci / publish-over-ssh-plugin

https://plugins.jenkins.io/publish-over-ssh/
117 stars 150 forks source link

[JENKINS-67963] Add option to save bandwidth and resources, which are wasted unnecessarily #65

Closed jira-importer closed 2 years ago

jira-importer commented 2 years ago

At this moment, the plugin always sends full build artifacts (tons of media files and etc) over each build, which is an absolute overkill and waste of:

so, whenever you have a chance, please implement this trivial things - before sending the artifact files, compare them to last sent file's md5 (or whatever) signature, and if content is not changed, don't resend. You can leave that 'option' by default turned off, but at least, give us way to enable that option.


Originally reported by selnomeria, imported from: Add option to save bandwidth and resources, which are wasted unnecessarily
  • status: Open
  • priority: Minor
  • resolution: Unresolved
  • imported: 2022/01/10
parvit commented 2 years ago

Hi @ttodua, i've come across the bounty and i've worked on this the last couple of days, and i think it should fulfill the request. Please see my change at https://github.com/parvit/publish-over-ssh-plugin/tree/JENKINS-67963 and tell me if there is anything else necessary.

Regards.

ttodua commented 2 years ago

@parvit Hey parvit, nice to see you worked on it. As i doubt i will not be able to test out this within this week (our Jenkins is offline) and dont want to delay your bounty.

if you wish, I can release the bounty now onto you, but if I will find a relatively simple bug/issue later, will you be able to address that? of course, I dont mean adding other things, just that what you worked on.

also if you can, make a PR to this main repository, so Jenkins devs (@gmcdonald) can merge too and thus issue will be closed, which will allow you to request bounty.

parvit commented 2 years ago

sure

parvit commented 2 years ago

@ttodua Hi, is there any update? thanks.

gmcdonald commented 2 years ago

Thanks @parvit and @ttodua - I dont want to hold this up, but I do want to test everything before merging. I'll try and get this done tomorrow

parvit commented 2 years ago

@ttodua @gmcdonald Hi, is there anything i can help with to speed up the test? If you're busy i can do it for you and post a report.

gmcdonald commented 2 years ago

I downloaded the PR into a new branch and tested. It all seems to work fine. @parvit it would not hurt if you also want to provide some test results as confirmation and then I will merge.

One thing to note - the new feature checkbox is only available from the Admin - > Configure area of Jenkins and so it not configurable at job/user level. The original issue did not make it clear whether or not this is needed. In my opinion it is a 'nice to have' - perhaps extend to be able to override the Admin setting at job level, I will leave that to others to open a new Issue if they think it beneficial.

I am happy to merge, but if @parvit is able to provide a test also then I'd prefer that 2nd opinion.

parvit commented 2 years ago

sure, later i'll provide logs of the tests.

parvit commented 2 years ago

I've updated the PR with the test logs.

gmcdonald commented 2 years ago

PR merged, thank you.

parvit commented 2 years ago

@ttodua @gmcdonald Thank you.