Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Check if the script is in the remote host and copy if not #12

Merged
merged 15 commits into from
Jun 30, 2022

Conversation

marksabbath
Copy link
Contributor

JIRA Ticket

What Are We Doing Here

This PR introduces a verification when the post script is set to run. The approach used is: 1. Check if the script exists in the remote host. If exists, run the version in the remote host, if it doesn't exists in the remote host, it will check if the script exists in the repository and rsync it to the remote host and execute.

@marksabbath marksabbath marked this pull request as ready for review June 24, 2022 12:32
@marksabbath marksabbath requested a review from a team as a code owner June 24, 2022 12:32
Copy link
Member

@apmatthews apmatthews left a comment

Choose a reason for hiding this comment

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

Something I noticed in testing was that it's not possible to update the script if you don't include any kind of self-destruct code (i.e. rm ./path/to/script.sh). The script will remain on the server, which causes the rsync to be skipped on the next deploy.

Would it make sense to just rsync the script regardless of whether the file already exists on the remote?

@marksabbath
Copy link
Contributor Author

Something I noticed in testing was that it's not possible to update the script if you don't include any kind of self-destruct code (i.e. rm ./path/to/script.sh). The script will remain on the server, which causes the rsync to be skipped on the next deploy.

Would it make sense to just rsync the script regardless of whether the file already exists on the remote?

Hey @apmatthews we've discussed that approach in our standup. @alexanderzuniga proposed that we follow this path implemented but we definitely can revisit it and chat about that when you are back. There's no rush to merge this PR.

Copy link
Contributor

@daniel-savo daniel-savo left a comment

Choose a reason for hiding this comment

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

LGTM

@marksabbath marksabbath merged commit 34e41e0 into main Jun 30, 2022
@marksabbath marksabbath deleted the update/check-script-remote branch June 30, 2022 14:21
@github-actions github-actions bot mentioned this pull request Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants