Conversation
packages/cache/src/options.ts
Outdated
| } | ||
| } | ||
|
|
||
| const customDownloadTimeoutMins = process.env['CACHE_DOWNLOAD_TIMEOUT_MINS'] |
There was a problem hiding this comment.
Shouldn't we name the env variable as SEGMENT_DOWNLOAD_TIMEOUT_MINS to be more accurate? This timeout will not necessary make the whole download timeout as it may be non-first segment which gets stuck.
There was a problem hiding this comment.
Since the users are not aware of the internal working of the download client, hence kept it a generic one that everyone would understand. If we want to keep it SEGMENT_DOWNLOAD_TIMEOUT_MINS, we'll have to also explain more about what a segment is, how the download segment size differs in 32 bit and 64 bit systems, etc.
There was a problem hiding this comment.
That's a good point. It's just that customer might come back complaining that I set a timeout and the download timed out only after a longer time period.
If it is helpful, we can document the process to download in segments and mention the timeout there. But agree that this is additional complexity.
There was a problem hiding this comment.
Will work on the documenting part then...
There was a problem hiding this comment.
@aparna-ravindra @anuragc617 any thoughts here?
There was a problem hiding this comment.
CACHE_DOWNLOAD_TIMEOUT_MINS This is not complete cache download timeout, It is timeout for 2GB Should we rename to reflect this ? like CACHE_DOWNLOAD_TIMEOUT_PER_GB and we can compute the timeout for 2gb internally. This way we might not need to document internal working as well
There was a problem hiding this comment.
Does readme work for the same? @tiwarishub
Added an env var
SEGMENT_DOWNLOAD_TIMEOUT_MINSthat overrides the 1 hour default timeout for decreasing the cache download timeout.A successful run