Feature/zenko 18/metadata proxy server #498

ploki · 2018-06-06T23:35:21Z

replaces #462

bert-e · 2018-06-06T23:35:22Z

Hello ploki,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get
information on this process.

Status report is not available.

bert-e · 2018-06-06T23:35:24Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

2 peers

ironman-machine · 2018-06-08T21:57:39Z

PR has been updated. Reviewers, please be cautious.

ghost · 2018-06-08T21:57:42Z

There were the following issues with this Pull Request

Commit: 8f68d62
- ✖ message may not be empty
  , - ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.

🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

ironman-machine · 2018-06-08T22:15:26Z

PR has been updated. Reviewers, please be cautious.

Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)

ironman-machine · 2018-06-11T22:25:31Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-06-11T23:13:11Z

PR has been updated. Reviewers, please be cautious.

rahulreddy

I like your usage of RequestDispatcher in the tests.

rahulreddy · 2018-06-12T19:30:39Z

lib/storage/metadata/proxy/BucketdRoutes.js

+
+    _putObject(req, res, bucketName, objectName, objectValue, params, logger) {
+        return this._metadataWrapper.client.putObject(
+            bucketName, objectName, JSON.parse(objectValue),


JSON.parse will call cause a server crash if the JSON is malformed. I recommend a try...catch for safe parsing.

rahulreddy · 2018-06-12T19:50:25Z

lib/storage/metadata/proxy/BucketdRoutes.js

+     * the purpose of this server. The `value' fields are stringified twice and
+     * have to be deserialized here before being sent to the client.
+     */
+    _destringifyValues(data) {


Is this method being used anywhere or is it a placeholder for future use?

rahulreddy · 2018-06-12T19:50:45Z

lib/storage/metadata/proxy/BucketdRoutes.js

+     */
+    _destringifyValues(data) {
+        const newContents = data.Contents.map(entry => {
+            const value = JSON.parse(entry.value);


I recommend safe json parsing as above.

rahulreddy · 2018-06-12T19:52:48Z

lib/storage/metadata/proxy/BucketdRoutes.js

+     *
+     * @param {http.IncomingMessage} req - request being processed
+     * @param {http.OutgoingMessage} res - response associated to the request
+     * @param {object} uriComponents -


You can list here the properties expected to be used in the object.

rahulreddy · 2018-06-12T20:04:01Z

lib/storage/metadata/proxy/utils.js

+ * @param {string} uri - uri part of the received request
+ * @param {werelogs.Logger} logger -
+ * @return {object} ret - contains up to 4 string properties,
+ * @return {string} ret.namespace


Can you add a one-liner describing what namespace and context are?

rahulreddy · 2018-06-12T20:05:24Z

lib/storage/metadata/proxy/utils.js

+        const bucketIndex = pathname.indexOf('/', typeIndex + 1);
+        const objectIndex = pathname.indexOf('/', bucketIndex + 1);
+
+        if (typeIndex === -1 || typeIndex === pathname.length - 1) {


Suggestion: If you abstract pathname.length - 1 into a var, it will be more readable.

rahulreddy · 2018-06-12T20:06:39Z

lib/storage/metadata/proxy/utils.js

+        };
+    } catch (ex) {
+        logger.error('Invalid URI: failed to parse',
+                     { uri, error: ex, errorStack: ex.stack });


ex.message is worth logging too.

rahulreddy · 2018-06-12T22:16:56Z

lib/storage/metadata/proxy/utils.js

+    request.on('data', data => {
+        body.push(data);
+        bodyLen += data.length;
+    }).on('error', cb).on('end', () => {


http.IncomingMessage emits close if the client abruptly disconnects. We need to add a listener and destroy the stream/cleanup when the close event is emitted. This was the source of our TCP socket leak issue in January.

rahulreddy · 2018-06-12T22:25:51Z

tests/functional/metadataProxy/routesToMem.js

+                                   }),
+            next => dispatcher.delete(`/default/bucket/${Bucket}/test1`,
+                                      err => next(err)),
+        ], err => done(err));


Minor style comment: In instances like this err => done(err) you can pass the callback directly done.

Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)

ironman-machine · 2018-06-13T00:34:13Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-06-13T00:36:40Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-06-13T00:41:16Z

PR has been updated. Reviewers, please be cautious.

ironman-machine · 2018-06-13T01:06:39Z

PR has been updated. Reviewers, please be cautious.

Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)

ironman-machine · 2018-06-13T17:06:40Z

PR has been updated. Reviewers, please be cautious.

bert-e · 2018-06-13T20:47:11Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/8.0

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of Release Engineering will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of Release Engineering now.

bert-e · 2018-06-13T20:47:15Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/8.0

Please check the status of the associated issue ZENKO-18.

Goodbye ploki.

ploki mentioned this pull request Jun 6, 2018

ft: ZENKO-18: Metadata Proxy Server #462

Closed

ploki requested a review from rahulreddy June 6, 2018 23:38

ploki force-pushed the feature/ZENKO-18/metadata-proxy-server branch from 8f68d62 to 670f1c6 Compare June 8, 2018 22:15

vrancurel previously approved these changes Jun 8, 2018

View reviewed changes

philipyoo previously approved these changes Jun 8, 2018

View reviewed changes

alexanderchan-scality previously approved these changes Jun 9, 2018

View reviewed changes

ploki force-pushed the feature/ZENKO-18/metadata-proxy-server branch from a3cf6d4 to b7cf797 Compare June 11, 2018 23:13

ssalaues previously approved these changes Jun 12, 2018

View reviewed changes

rahulreddy requested changes Jun 12, 2018

View reviewed changes

ploki force-pushed the feature/ZENKO-18/metadata-proxy-server branch from 8799960 to e4e3d04 Compare June 13, 2018 00:36

ploki force-pushed the feature/ZENKO-18/metadata-proxy-server branch from e4e3d04 to 2e02fe1 Compare June 13, 2018 00:41

ploki force-pushed the feature/ZENKO-18/metadata-proxy-server branch from 2e02fe1 to de77400 Compare June 13, 2018 01:06

rahulreddy previously approved these changes Jun 13, 2018

View reviewed changes

FT: Metadata Proxy Server

32c2a6f

ploki force-pushed the feature/ZENKO-18/metadata-proxy-server branch from de77400 to 32c2a6f Compare June 13, 2018 17:06

rahulreddy approved these changes Jun 13, 2018

View reviewed changes

ssalaues approved these changes Jun 13, 2018

View reviewed changes

bert-e merged commit 32c2a6f into development/8.0 Jun 13, 2018

ploki deleted the feature/ZENKO-18/metadata-proxy-server branch June 13, 2018 21:48

snyk-bot mentioned this pull request Apr 1, 2021

[Snyk] Security upgrade azure-storage from 2.1.0 to 2.10.2 #1381

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/zenko 18/metadata proxy server #498

Feature/zenko 18/metadata proxy server #498

ploki commented Jun 6, 2018

bert-e commented Jun 6, 2018

bert-e commented Jun 6, 2018

ironman-machine commented Jun 8, 2018

ghost commented Jun 8, 2018

ironman-machine commented Jun 8, 2018

ironman-machine commented Jun 11, 2018

ironman-machine commented Jun 11, 2018

rahulreddy left a comment

rahulreddy Jun 12, 2018

rahulreddy Jun 12, 2018

rahulreddy Jun 12, 2018

rahulreddy Jun 12, 2018

rahulreddy Jun 12, 2018

rahulreddy Jun 12, 2018

rahulreddy Jun 12, 2018

rahulreddy Jun 12, 2018

rahulreddy Jun 12, 2018

ironman-machine commented Jun 13, 2018

ironman-machine commented Jun 13, 2018

ironman-machine commented Jun 13, 2018

ironman-machine commented Jun 13, 2018

ironman-machine commented Jun 13, 2018

bert-e commented Jun 13, 2018

bert-e commented Jun 13, 2018

Feature/zenko 18/metadata proxy server #498

Feature/zenko 18/metadata proxy server #498

Conversation

ploki commented Jun 6, 2018

bert-e commented Jun 6, 2018

Hello ploki,

bert-e commented Jun 6, 2018

Waiting for approval

ironman-machine commented Jun 8, 2018

ghost commented Jun 8, 2018

ironman-machine commented Jun 8, 2018

ironman-machine commented Jun 11, 2018

ironman-machine commented Jun 11, 2018

rahulreddy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ironman-machine commented Jun 13, 2018

ironman-machine commented Jun 13, 2018

ironman-machine commented Jun 13, 2018

ironman-machine commented Jun 13, 2018

ironman-machine commented Jun 13, 2018

bert-e commented Jun 13, 2018

In the queue

bert-e commented Jun 13, 2018