Skip to content
This repository has been archived by the owner on Mar 14, 2023. It is now read-only.

Add show-progress and show-stats flags to enable logging during rsync #240

Closed

Conversation

utkarsh-dixit
Copy link

@utkarsh-dixit utkarsh-dixit commented Jul 8, 2019

This would allow the user to see the syncing progress while pushing the files to remote server.

Why this PR?

  1. I recently started using shipit and one of the problems that I encountered while deploying my files to my server is rsync getting closed because of ssh timeout trigger due to no activity. Because of this issue, code is only deployed once out of 5 times.

  2. Also, because of some of my network issues it takes a lot of time for rsync to complete and I had no idea if anything was going on or not due to inactivity.

Because of the above two reasons I thought of adding a "--show-progress" and "--show-stats" to shipit so that the user can track the progress. The progress activity would also stop the triggering of ssh connection timeout. (https://unix.stackexchange.com/a/68784).

Use-cases:

  1. Fixes the issue of "rsync timed out".
  2. Allows the users to track syncing progress (especially users with poor network connection).
  3. Also when deploying large repository, progress bar would be helpful.

Usages:

  • Show Progress bar (Can use either "-p" or "--show-progress" flag)
    npx shipit production deploy --show-progress

  • Show Stats (Can use either "-s" or "--show-stats" flag)
    npx shipit production deploy --show-stats

packages/shipit-cli/src/Shipit.js Outdated Show resolved Hide resolved
packages/shipit-cli/src/cli.js Outdated Show resolved Hide resolved
@gregberge
Copy link
Member

Hello @utkarsh-dixit, it looks good for me, but could you please fix cases problem and linter issues.

packages/shipit-cli/src/cli.js Outdated Show resolved Hide resolved
packages/shipit-cli/src/cli.js Outdated Show resolved Hide resolved
@utkarsh-dixit utkarsh-dixit changed the title Add showProgress and showStats flags to enable logging during rsync Add show-progress and show-stats flags to enable logging during rsync Jul 8, 2019
@utkarsh-dixit
Copy link
Author

utkarsh-dixit commented Jul 9, 2019

@neoziro I have shifted this config to this.config.rsync since it's already there and this issue is also related to rsync.. While doing that I noticed that defaultOptions.rsync was not getting passed to ConnectionPool functions because it was getting overwritten.. See the code below:-

    const defaultOptions = {
      ignores: this.config && this.config.ignores ? this.config.ignores : [],
      rsync: this.config && this.config.rsync ? this.config.rsync : [],
    }
    const copyOptions = { ...defaultOptions, ...options }

And the options passed down by update::remoteCopy in shipit-deploy update is in the below lines.

      const options = _.get(shipit.config, 'deploy.remoteCopy') || {
        rsync: '--del',
      }

I have made some changes to Shipit.js so it doesn't get overwritten and instead be added to the existing config.. Is that okay, or was this logic intended?

@utkarsh-dixit
Copy link
Author

utkarsh-dixit commented Jul 9, 2019

I noticed lot of tests are failing because of this change. They are expecting the functions to overwrite the defaultOptions.rsync. I have used loadash unique() to remove duplicates, and the tests have passed now.

@utkarsh-dixit
Copy link
Author

I was looking at the issue board and found out the issue similar to the one I encountered (#209). This pull request would also solve #209

Copy link
Member

@gregberge gregberge left a comment

Choose a reason for hiding this comment

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

I know it could be difficult but could you add some tests to cover the new feature?

packages/shipit-cli/package.json Outdated Show resolved Hide resolved
@utkarsh-dixit
Copy link
Author

@neoziro Any updates regarding this pull request?

@gregberge
Copy link
Member

@utkarsh-dixit I don't know the goal, why not creating shipit instance with options:

new Shipit({ rsync: ['--progress'] })

What is the difference?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants