BM-2474: set minio TTL to 1 day, enable max order expiry config#1660
BM-2474: set minio TTL to 1 day, enable max order expiry config#1660austinabell merged 4 commits intomainfrom
Conversation
austinabell
requested review from
a team,
Wollac,
capossele,
ec2,
jonastheis,
nahoc,
unizarr,
willemolding and
zeroecco
as code owners
github-actions
bot
changed the title
| } | ||
|
|
||
| // Check if the order expiry exceeds the max allowed duration | ||
| if let Some(max_expiry) = config.max_order_expiry_secs { |
There was a problem hiding this comment.
I'm a bit worried on its potential misuses. Do we know looking at the market how long the expiration is? I mean is a day enough for instance? I feel this type of information would be useful to provide to educate the user on how to set this and what values are safe/recommended
There was a problem hiding this comment.
and potentially we could warn if it gets set to a uncommon value?
There was a problem hiding this comment.
On the requestor side, it would warn if a request expiry is past that deadline because it runs this function now.
I do get the concern, that there may be a reason for an order with expiries (lock timeout and expiry) longer than a day, but at least to me seems like a more reasonable default than allowing unbounded on the prover
There was a problem hiding this comment.
or do you mean user being a prover and what they should override this config to if they want?
There was a problem hiding this comment.
yes that, It could also just live on the docs
aws/minio TTL is a bit tricky to test, but confirmed that running this sets the policy with: ``` docker compose exec minio mc ilm rule ls local/workflow ... ┌────────────────────────────────────────────────────────────────────────────────────────┐ │ Expiration for latest version (Expiration) │ ├──────────────────────┬─────────┬─────────┬──────┬────────────────┬─────────────────────┤ │ ID │ STATUS │ PREFIX │ TAGS │ DAYS TO EXPIRE │ EXPIRE DELETEMARKER │ ├──────────────────────┼─────────┼─────────┼──────┼────────────────┼─────────────────────┤ │ d6ailbe6vpls76nmb770 │ Enabled │ inputs/ │ - │ 1 │ false │ └──────────────────────┴─────────┴─────────┴──────┴────────────────┴─────────────────────┘ ``` Setting the policy through aws sdk APIs are broken with minio, which is why `mc` CLI is used -- so this wouldn't set a TTL if not using minio Alternative to this is having a script run on some interval, similar to #1244 (comment) -- but this is intended to avoid having an extra process running (at the cost that expiries can only be configured based on number of days). Do not feel strongly about this, if we want to have more granular control over the cleanup deletions, happy to swap this to a loop script.
4 participants
aws/minio TTL is a bit tricky to test, but confirmed that running this sets the policy with:
Setting the policy through aws sdk APIs are broken with minio, which is why
mcCLI is used -- so this wouldn't set a TTL if not using minioAlternative to this is having a script run on some interval, similar to #1244 (comment) -- but this is intended to avoid having an extra process running (at the cost that expiries can only be configured based on number of days). Do not feel strongly about this, if we want to have more granular control over the cleanup deletions, happy to swap this to a loop script.