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

fix cronjob sync escaping issue with env vars #109

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

bramstroker
Copy link
Contributor

@bramstroker bramstroker commented Jun 27, 2024

Environment variables were disappearing after crontab is synced.
Should be fixed now by properly escaping.

Assuming cron job which gets deployed.

MAGENTO_DOCROOT=/data/web/apps/mydomain.com/current
MAGENTO_PHPBIN=php

0 4 * * * cd $MAGENTO_DOCROOT && $MAGENTO_PHPBIN ./bin/magento foo-bar

After deployment on the server crontab is as follows:

MAGENTO_DOCROOT=/data/web/apps/mydomain.com/current
MAGENTO_PHPBIN=php

0 4 * * * cd && ./bin/magento foo-bar

This PR fixes the issue.

Copy link
Collaborator

@tdgroot tdgroot left a comment

Choose a reason for hiding this comment

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

Good find, LGTM!

@tdgroot tdgroot merged commit 9754958 into ByteInternet:master Jun 28, 2024
5 of 8 checks passed
@bramstroker
Copy link
Contributor Author

Hi @tdgroot Thanks for merging!
I see a github action regarding integration check is failing?
And when is this expected to be released in a new tag?

This is the final roadblock for us regarding the hypernode deploy which we have implemented for all our clients migrating to Hypernode platform.

@tdgroot
Copy link
Collaborator

tdgroot commented Jul 2, 2024

Hi @bramstroker, I'll draft a release tomorrow! :)

@tdgroot
Copy link
Collaborator

tdgroot commented Jul 4, 2024

Hi @bramstroker,

The release (https://github.com/ByteInternet/hypernode-deploy/releases/tag/3.6.2) has been published!

It took me a bit longer because there were some regressions in the master branch.

@bramstroker
Copy link
Contributor Author

@tdgroot no worries, thanks for your efforts!

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

Successfully merging this pull request may close these issues.

2 participants