š use SUR date instead of the infostore one to have an update LastModā¦#6121
š use SUR date instead of the infostore one to have an update LastModā¦#6121DarkIsDude wants to merge 1 commit intodevelopment/9.1from
Conversation
Hello darkisdude,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
| // Extract the last modified date, but do not include it when computing | ||
| // the file's ETag (md5) | ||
| const modified = fileToBuild.value.LastModified; | ||
| const modified = bucketMetrics?.date || fileToBuild.value.LastModified; |
There was a problem hiding this comment.
Should we remove fileToBuild.value.LastModified. IMO yes ?
There was a problem hiding this comment.
we would still need a "default" value though, we cannot pass "undefined" as the date?
There was a problem hiding this comment.
We don't call the SUR backend on this route. So we can't use the new date. It's not an issue as Veeam is not doing head but can lead to weird behaviour. We have the same "issue" in the list.
There was a problem hiding this comment.
Please be advised that the UI is using the List/Head...
There was a problem hiding this comment.
-
HEAD: Clear-cut: add the UtilizationService call for capacity.xml, same pattern as GET. HEAD is contractually required to return the same headers as GET ā inconsistent Last-Modified between GET and HEAD would be a spec violation.
-
LIST: This is the tricky one. Without calling UtilizationService, there are really only two options:
- Keep the metadata date ā Stale, and actively harmful: if the LIST LastModified for capacity.xml doesn't change, Veeam may cache the entry and skip re-fetching via GET, which defeats the entire purpose of using the metrics date.
- Use new Date().toISOString() in LIST for capacity.xml ā Always returns current time. This signals to Veeam: "this file is always potentially fresh, always re-fetch via GET/HEAD." Since GET/HEAD now return the accurate bucketMetrics.date, Veeam gets the right value on the actual fetch. Capacity metrics do change constantly, so this is semantically honest.
Option 2 is the pragmatic fix: no extra service call, no stale caching problem, and the accurate date is still served on GET/HEAD.
There was a problem hiding this comment.
nothing currently implemented: this is the same code as before, right?
Fetching metrics in HEAD and returning current date in LIST seem ok to me, though Veeam may still do weird things: e.g. it may not like that LIST returns a date which is newer than the GET/HEAD call it makes next... Could we make a call to utilization api for LIST as well?
Codecov Reportā
All modified and coverable lines are covered by tests. Additional details and impacted files
... and 4 files with indirect coverage changes @@ Coverage Diff @@
## development/9.1 #6121 +/- ##
===================================================
+ Coverage 83.79% 83.84% +0.04%
===================================================
Files 191 191
Lines 12350 12349 -1
===================================================
+ Hits 10349 10354 +5
+ Misses 2001 1995 -6
Flags with carried forward coverage won't be shown. Click here to find out more. š New features to boost your workflow:
|
DarkIsDude
force-pushed
the
bugfix/CLDSRV-878/use-sur-date
branch
from
bb7499d to
beee7a8
Compare
ā¦ified date Issue: CLDSRV-878
DarkIsDude
force-pushed
the
bugfix/CLDSRV-878/use-sur-date
branch
from
beee7a8 to
d9a5aed
Compare
| return UtilizationService.getUtilizationMetrics('bucket', bucketKey, null, {}, (err, bucketMetrics) => { | ||
| if (err) { | ||
| // Handle errors from UtilizationService/scubaclient | ||
| // axios errors have status in err.response.status |
There was a problem hiding this comment.
I think this comment was actually useful : we never have err.response.status in our code base, kind of weird to see it checked in addition to the more usual (in our code!) err.statusCode and err.code
| const date = moment(); | ||
| const expectedRetainUntilDate | ||
| = date.add(mockConfigWithYears.years * 365, 'days'); | ||
| = date.add(mockConfigWithYears.years * 365 * 86400000, 'ms'); |
There was a problem hiding this comment.
why this change? 'days' is more readable than 864000000ms...
3 participants
Issue: CLDSRV-878
Pull request template
Description
https://scality.atlassian.net/browse/CLDSRV-631 this PR introduce a regression. The associated cronjob was removed https://github.com/scality/zenko-operator/pull/540/changes#diff-c76a6b4555478f74e4c871dbf370d4e6e5ae54c4b8590b0581b1cc0894e83ae5R55 but the date used is still the one associated with it. The goal of this PR is to use the date from SUR.
With the fix
< last-modified: Fri, 27 Mar 2026 16:04:16 GMTWithout the fix
< last-modified: Wed, 25 Mar 2026 14:12:11 GMT