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

Renamed cache_mem_size to cache_index_size #103

Closed
wants to merge 1 commit into from

Conversation

Crabbey
Copy link
Contributor

@Crabbey Crabbey commented Mar 30, 2020

No description provided.

@@ -4,9 +4,16 @@ set -e
# Preprocess UPSTREAM_DNS to allow for multiple resolvers using the same syntax as lancache-dns
UPSTREAM_DNS="$(echo -n "${UPSTREAM_DNS}" | sed 's/[;]/ /g')"

if [ ! -z ${CACHE_MEM_SIZE} ]; then
Copy link
Member

Choose a reason for hiding this comment

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

As per monolithic this should be done in a deprecation file so all the checks happen in the same place (I'd perhaps suggest 00_generic_deprecation.sh to avoid naming clashes)

https://github.com/lancachenet/monolithic/blob/master/overlay/hooks/entrypoint-pre.d/00_deprecation.sh

Copy link
Member

Choose a reason for hiding this comment

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

At the moment it can probably just warning rather than error, then EXPORT CACHE_INDEX_SIZE up to the controlling shell?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure you can export a variable from one script to the parent shell - unless this is a magic bit of supervisord functionality?
Could move the warning out to a separate script, and leave the action in place?

Copy link
Member

Choose a reason for hiding this comment

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

Kinda forgot about this!

I think the best option with fresh eyes is to do the transfer here so it can be used in the script, and then put the warning message into deprecation.sh but without the abort so it will stilll continue for the moment.

We would also need to update the website and docker-compose (I don't think we have PR's waiting for those do we?

VibroAxe added a commit to lancachenet/monolithic that referenced this pull request Oct 9, 2021
Ported PR lancachenet/generic#103 from @Crabbey
Changes CACHE_MEM_SIZE to CACHE_INDEX_SIZE but maintains backwards compatiblity through deprecation.sh
VibroAxe added a commit to lancachenet/monolithic that referenced this pull request Oct 9, 2021
@VibroAxe
Copy link
Member

VibroAxe commented Oct 9, 2021

Merged via lancachenet/monolithic#133

@VibroAxe VibroAxe closed this Oct 9, 2021
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.

2 participants