Skip to content

Improvement/s3 utils 191 #337

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: development/1.16
Choose a base branch
from
Open

Improvement/s3 utils 191 #337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ca66c5d
bump arsenal to latest version
benzekrimaha May 5, 2025
bdc84f9
Code and tests update
benzekrimaha May 6, 2025
1c533be
project version bump
benzekrimaha May 6, 2025
8f20ab6
test addition for get transient
benzekrimaha May 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion CountItems/CountWorker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { BucketInfo } = require('arsenal').models;
const monitoring = require('../utils/monitoring');
const { deserializeBigInts, serializeBigInts } = require('./utils/utils');

const PENSIEVE = 'PENSIEVE';
class CountWorker {
constructor(params) {
this.log = params.log;
Expand All @@ -27,6 +28,40 @@ class CountWorker {
return this.client.setup(callback);
}

getIsTransient(bucketInfo, cb) {
const locConstraint = bucketInfo.getLocationConstraint();

if (this.client.isLocationTransient) {
this.client.isLocationTransient(locConstraint, this.log, cb);
return;
}
this.pensieveLocationIsTransient(locConstraint, cb);
}

pensieveLocationIsTransient(locConstraint, cb) {
const overlayVersionId = 'configuration/overlay-version';

async.waterfall([
next => this.client.getObject(PENSIEVE, overlayVersionId, null, this.log, next),
(version, next) => {
const overlayConfigId = `configuration/overlay/${version}`;
return this.client.getObject(PENSIEVE, overlayConfigId, null, this.log, next);
},
], (err, res) => {
if (err) {
this.log.error('error getting configuration overlay', {
method: 'pensieveLocationIsTransient',
error: err,
});
return cb(err);
}
const isTransient =
Boolean(res.locations[locConstraint].isTransient);

return cb(null, isTransient);
});
}

countItems(bucketInfoObj, callback) {
if (!this.client.client) {
return callback(new Error('NotConnected'));
Expand All @@ -40,7 +75,7 @@ class CountWorker {
const bucketName = bucketInfo.getName();
this.log.info(`${process.pid} handling ${bucketName}`);
return async.waterfall([
next => this.client._getIsTransient(bucketInfo, this.log, next),
next => this.getIsTransient(bucketInfo, next),
(isTransient, next) => this.client.getObjectMDStats(bucketName, bucketInfo, isTransient, this.log, next),
], (err, results) => {
monitoring.bucketsCount.inc({ status: err ? 'error' : 'success' });
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "s3utils",
"version": "1.15.8",
"version": "1.16.0",
"engines": {
"node": ">= 22"
},
Expand Down Expand Up @@ -28,7 +28,7 @@
"dependencies": {
"@senx/warp10": "^2.0.3",
"JSONStream": "^1.3.5",
"arsenal": "git+https://github.com/scality/arsenal#8.2.8",
"arsenal": "git+https://github.com/scality/arsenal#22a5462d7d9bdec0d21a0e9fb484594af35c1cee",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please note that this will be switched to the arsenal adequate version once the current in review PR with a fix is merged: scality/Arsenal#2371

"async": "^3.2.6",
"aws-sdk": "^2.1691.0",
"bucketclient": "scality/bucketclient#8.2.2",
Expand Down
2 changes: 1 addition & 1 deletion tests/mocks/mongoClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ module.exports = {
},
setup: jest.fn(),
close: jest.fn(),
_getIsTransient: jest.fn(),
getObjectMDStats: jest.fn(),
getObject: jest.fn(),
getBucketInfos: jest.fn(),
updateStorageConsumptionMetrics: jest.fn(),
getUsersBucketCreationDate: jest.fn(),
Expand Down
49 changes: 39 additions & 10 deletions tests/unit/CountItems/CountWorker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ describe('CountItems::CountWorker', () => {
mongoMock.close.mockReset();
mongoMock.setup.mockReset();
mongoMock.client.isConnected.mockReset();
mongoMock._getIsTransient.mockReset();
mongoMock.getObjectMDStats.mockReset();
});

Expand All @@ -22,7 +21,6 @@ describe('CountItems::CountWorker', () => {
setup: [null],
close: [],
isConnected: false,
getIsTransient: [],
getObjectMDStats: [],
},
incomingMessage: {
Expand All @@ -47,7 +45,6 @@ describe('CountItems::CountWorker', () => {
setup: [new Error('failed setup')],
close: [],
isConnected: false,
getIsTransient: [],
getObjectMDStats: [],
},
incomingMessage: {
Expand Down Expand Up @@ -98,7 +95,6 @@ describe('CountItems::CountWorker', () => {
setup: [],
close: [new Error('failed teardown')],
isConnected: true,
getIsTransient: [],
getObjectMDStats: [],
},
incomingMessage: {
Expand All @@ -124,7 +120,6 @@ describe('CountItems::CountWorker', () => {
setup: [],
close: [],
isConnected: true,
getIsTransient: [null, true],
getObjectMDStats: [null, { value: 42 }],
},
incomingMessage: {
Expand All @@ -151,7 +146,6 @@ describe('CountItems::CountWorker', () => {
setup: [],
close: [],
isConnected: true,
getIsTransient: [null, true],
getObjectMDStats: [new Error('count error')],
},
incomingMessage: {
Expand Down Expand Up @@ -180,13 +174,11 @@ describe('CountItems::CountWorker', () => {
sendFn: testSendFn,
client: mongoMock,
});
w.getIsTransient = jest.fn((bucketInfo, cb) => cb(null, true));
Copy link
Contributor

Choose a reason for hiding this comment

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

just mocking getIsTransient means we basically do not execute this code...

  • Best to have some test exercising the "whole" code
  • If needed, we may use a mock (in some tests, ideally not all) to simulate specific conditions/results/...
  • In any case, need extra tests cases to verify the new functions behave somewhat correctly (esp. do not crash) and the information ("is transient") is used appropriately

mongoMock.setup.mockImplementationOnce(cb => cb(...tc.mock.setup));
mongoMock.close.mockImplementationOnce(cb => cb(...tc.mock.close));
mongoMock.client.isConnected
.mockImplementationOnce(() => tc.mock.isConnected);
mongoMock._getIsTransient.mockImplementationOnce(
(_a, _b, cb) => cb(...tc.mock.getIsTransient),
);
mongoMock.getObjectMDStats.mockImplementationOnce(
(_a, _b, _c, _d, cb) => cb(...tc.mock.getObjectMDStats),
);
Expand All @@ -213,15 +205,52 @@ describe('CountItems::CountWorker', () => {
_creationDate: Date.now().toString(),
website: { indexDocument: 'index.html' },
};
w.getIsTransient = jest.fn((bucketInfo, cb) => cb(null, true));
mongoMock.setup.mockImplementationOnce(cb => cb());
mongoMock.close.mockImplementationOnce(cb => cb());
mongoMock.client.isConnected.mockImplementationOnce(() => false);
mongoMock._getIsTransient.mockImplementationOnce((_a, _b, cb) => cb(null, true));
mongoMock.getObjectMDStats.mockImplementationOnce((_a, _b, _c, _d, cb) => cb(null, { value: 42 }));
w.countItems(bucketInfo, (err, results) => {
expect(err).toBeNull();
expect(results).toEqual({ value: 42 });
done();
});
});

test('should use real getIsTransient and not crash', done => {
const testSendFn = jest.fn();
let getObjectCall = 0;
const w = new CountWorker({
log: new DummyLogger(),
sendFn: testSendFn,
client: mongoMock,
});
mongoMock.setup.mockImplementationOnce(cb => cb());
mongoMock.close.mockImplementationOnce(cb => cb());
mongoMock.client.isConnected.mockImplementationOnce(() => false);
mongoMock.getObjectMDStats.mockImplementationOnce((_a, _b, _c, _d, cb) => cb(null, { value: 42 }));
mongoMock.getObject = jest.fn((_a, _b, _c, _d, cb) => {
getObjectCall += 1;
if (getObjectCall === 1) {
cb(null, 'v1');
} else {
cb(null, {
locations: {
undefined: { isTransient: true },
},
});
}
});
const bucketInfo = {
_name: 'test-bucket',
_owner: 'any',
_ownerDisplayName: 'any',
_creationDate: Date.now().toString(),
};
w.countItems(bucketInfo, (err, results) => {
expect(err).toBeNull();
expect(results).toEqual({ value: 42 });
done();
});
});
});
32 changes: 14 additions & 18 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@
fast-xml-parser "^5.0.7"
tslib "^2.8.1"

"@azure/identity@^4.5.0":
"@azure/identity@^4.5.0", "@azure/identity@^4.9.1":
version "4.9.1"
resolved "https://registry.yarnpkg.com/@azure/identity/-/identity-4.9.1.tgz#ee4b9435f1b96bea5985e7dec989760a67d9a119"
integrity sha512-986D7Cf1AOwYqSDtO/FnMAyk/Jc8qpftkGsxuehoh4F85MhQ4fICBGX/44+X1y78lN4Sqib3Bsoaoh/FvOGgmg==
Expand Down Expand Up @@ -1436,7 +1436,7 @@ agent-base@^7.1.0, agent-base@^7.1.2:
resolved "https://registry.yarnpkg.com/agent-base/-/agent-base-7.1.3.tgz#29435eb821bc4194633a5b89e5bc4703bafc25a1"
integrity sha512-jRR5wdylq8CkOe6hei19GGZnxM6rBGwFl3Bg0YItGDimvjGtAvdZk4Pu6Cl4u4Igsws4a1fd1Vq3ezrhn4KmFw==

agentkeepalive@^4.5.0:
agentkeepalive@^4.5.0, agentkeepalive@^4.6.0:
version "4.6.0"
resolved "https://registry.yarnpkg.com/agentkeepalive/-/agentkeepalive-4.6.0.tgz#35f73e94b3f40bf65f105219c623ad19c136ea6a"
integrity sha512-kja8j7PjmncONqaTsB8fQ+wE2mSU2DJ9D4XKoJ5PFWIdRMa6SLSN1ff4mOr4jCbfRSsxR4keIiySJU0N9T5hIQ==
Expand Down Expand Up @@ -1627,37 +1627,32 @@ arraybuffer.prototype.slice@^1.0.4:
optionalDependencies:
ioctl "^2.0.2"

"arsenal@git+https://github.com/scality/arsenal#8.2.8":
version "8.2.8"
resolved "git+https://github.com/scality/arsenal#a4136e5da67cadf3cff14de0daabd019bfd5f996"
"arsenal@git+https://github.com/scality/arsenal#22a5462d7d9bdec0d21a0e9fb484594af35c1cee":
version "8.2.14"
resolved "git+https://github.com/scality/arsenal#22a5462d7d9bdec0d21a0e9fb484594af35c1cee"
dependencies:
"@azure/identity" "^4.5.0"
"@azure/identity" "^4.9.1"
"@azure/storage-blob" "^12.25.0"
"@eslint/plugin-kit" "^0.2.3"
"@js-sdsl/ordered-set" "^4.4.2"
"@scality/hdclient" "^1.3.1"
"@types/async" "^3.2.24"
"@types/utf8" "^3.0.3"
JSONStream "^1.3.5"
agentkeepalive "^4.5.0"
agentkeepalive "^4.6.0"
ajv "6.12.3"
async "~2.6.4"
aws-sdk "^2.1691.0"
backo "^1.1.0"
base-x "3.0.8"
base62 "^2.0.2"
bson "^6.8.0"
debug "^4.3.7"
diskusage "^1.2.0"
fcntl "github:scality/node-fcntl#0.3.0"
httpagent scality/httpagent#1.1.0
https-proxy-agent "^7.0.5"
ioredis "^5.4.1"
https-proxy-agent "^7.0.6"
ioredis "^5.6.1"
ipaddr.js "^2.2.0"
joi "^17.13.3"
level "~5.0.1"
level-sublevel "~6.6.5"
mongodb "^6.11.0"
mongodb "^6.15.0"
node-forge "^1.3.1"
prom-client "^15.1.3"
simple-glob "^0.2.0"
Expand Down Expand Up @@ -3276,7 +3271,7 @@ http-proxy-agent@^7.0.0:
dependencies:
agentkeepalive "^4.5.0"

https-proxy-agent@^7.0.0, https-proxy-agent@^7.0.1, https-proxy-agent@^7.0.5:
https-proxy-agent@^7.0.0, https-proxy-agent@^7.0.1, https-proxy-agent@^7.0.5, https-proxy-agent@^7.0.6:
version "7.0.6"
resolved "https://registry.yarnpkg.com/https-proxy-agent/-/https-proxy-agent-7.0.6.tgz#da8dfeac7da130b05c2ba4b59c9b6cd66611a6b9"
integrity sha512-vK9P5/iUfdl95AI+JVyUuIcVtd4ofvtrOr3HNtM2yxC9bnMbEdp3x01OhQNnjb8IJYi38VlTE3mBXwcfvywuSw==
Expand Down Expand Up @@ -3379,7 +3374,7 @@ [email protected], ioctl@^2.0.2:
bindings "^1.5.0"
nan "^2.14.0"

ioredis@^5.4.1:
ioredis@^5.4.1, ioredis@^5.6.1:
version "5.6.1"
resolved "https://registry.yarnpkg.com/ioredis/-/ioredis-5.6.1.tgz#1ed7dc9131081e77342503425afceaf7357ae599"
integrity sha512-UxC0Yv1Y4WRJiGQxQkP0hfdL0/5/6YvdfOOClRgJ0qppSarkhneSa6UvkMkms0AkdGimSH3Ikqm+6mkMmX7vGA==
Expand Down Expand Up @@ -4815,7 +4810,7 @@ [email protected]:
mongodb-memory-server-core "10.1.2"
tslib "^2.7.0"

mongodb@^6.11.0, mongodb@^6.9.0:
mongodb@^6.11.0, mongodb@^6.15.0, mongodb@^6.9.0:
version "6.16.0"
resolved "https://registry.yarnpkg.com/mongodb/-/mongodb-6.16.0.tgz#2a7a1986ec151d9c738fc8ce4cf4324c3f728a2f"
integrity sha512-D1PNcdT0y4Grhou5Zi/qgipZOYeWrhLEpk33n3nm6LGtz61jvO88WlrWCK/bigMjpnOdAUKKQwsGIl0NtWMyYw==
Expand Down Expand Up @@ -6459,6 +6454,7 @@ word-wrap@^1.2.5:
integrity sha512-BN22B5eaMMI9UMtjrGd5g5eCYPpCPDUy0FJXbYsaT5zYxjFOckS53SQDE3pWkVoWpHXVb3BrYcEN4Twa55B5cA==

"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
name wrap-ansi-cjs
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
Expand Down