Changed sync timestamp location from file to directory#223
Changed sync timestamp location from file to directory#223
Conversation
corneliouzbett
left a comment
corneliouzbett
left a comment
There was a problem hiding this comment.
I think we should omit the check for existence of eip-odoo-openelis service as we don't it for the rest. Otherwise, the rest of changes looks fine.
scripts/start.sh
Outdated
| # Create the directory to store sync timestamp file for eip-odoo-openelis service | ||
| while read -r service; do | ||
| if [ "$service" == 'eip-odoo-openelis' ]; then | ||
| echo "Found eip-odoo-openelis service, configuring directory to store sync timestamp file" | ||
| export EIP_ODOO_OPENELIS_DATA_DIR=$DISTRO_PATH/data/eip_odoo_openelis | ||
| break | ||
| fi | ||
| done < /tmp/defined_services.txt |
There was a problem hiding this comment.
I think this is unnecessary. I think it's ok to always set EIP_ODOO_OPENELIS_DATA_DIR in regardless of whether the service exists or not.
There was a problem hiding this comment.
Then what would be its purpose if the service does not exist? I think it is prudent not to.
There was a problem hiding this comment.
I understand and I agree with you. However, we haven't done any checks for the other exports e.g. https://github.com/ozone-his/ozone-docker-compose/blob/main/scripts/utils.sh#L25-L52, plus start scripts are meant for demo and evaluation purposes. In production deployment, you would need to define these env vars manually based on which services to deploy. What I'm asking here is to be consistent with others
| export EIP_ODOO_OPENELIS_ROUTES_PATH=$DISTRO_PATH/binaries/eip-odoo-openelis | ||
| export EIP_SYNC_TASK_LAST_RUN_TS_FILE=$DISTRO_PATH/odoo_openelis_sync_ts.txt |
There was a problem hiding this comment.
We should keep this, let's rename to the new env var EIP_ODOO_OPENELIS_DATA_DIR
|
Updated the PR |
2 participants
Changed env var for sync timestamp location from file to directory