bugfix: S3C-2987 helper to get stream data as a JSON payload #1126

jonathan-gramain · 2020-06-24T22:46:44Z

Add a new helper function to get data from a stream as a JSON payload,
optionally validated against a joi schema.

Note: uses async/await, so updated the scality/Guidelines dependency
to please the linter

bert-e · 2020-06-24T22:46:46Z

Hello jonathan-gramain,

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

Status report is not available.

bert-e · 2020-06-24T22:46:55Z

Conflict

A conflict has been raised during the creation of
integration branch w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing with contents from w/7.8/bugfix/S3C-2987-helperForJsonStreamParsing
and development/8.1.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing origin/development/8.1
 $ git merge origin/w/7.8/bugfix/S3C-2987-helperForJsonStreamParsing
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing

miniscruff · 2020-06-24T22:53:41Z

lib/stream/utils.js

+            const contents = contentsChunks.join('');
+            try {
+                const parsedContents = JSON.parse(contents);
+                let validatedContents;


Could just resolve(value) and resolve(parsedContents) instead of creating this variable. Doesn't matter though, either way.

rahulreddy

One thing I suggest is instead of utils holding a couple of stream methods, export each method directly with its own file.
Instead of
- lib/stream/utils.js

module.exports = {
    readJSONStreamObject,
};

use
+lib/stream/readJSONStreamObject.js

module.exports = readJSONStreamObject;

It's easy to discover and use the methods this way.

bert-e · 2020-06-24T23:46:41Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge bugfix/S3C-2987-helperForJsonStreamParsing into
development/7.4
w/7.7/bugfix/S3C-2987-helperForJsonStreamParsing will be merged into development/7.7 (pull request INTEGRATION [PR#1126 > development/7.7] bugfix: S3C-2987 helper to get stream data as a JSON payload #1127)
w/7.8/bugfix/S3C-2987-helperForJsonStreamParsing will be merged into development/7.8 (pull request INTEGRATION [PR#1126 > development/7.8] bugfix: S3C-2987 helper to get stream data as a JSON payload #1128)
w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing will be merged into development/8.1 (pull request INTEGRATION [PR#1126 > development/8.1] bugfix: S3C-2987 helper to get stream data as a JSON payload #1129)

The following branches will NOT be impacted:

development/6.4
stabilization/7.4.6

Follow integration pull requests if you would like to be notified of
build statuses by email.

bert-e · 2020-06-24T23:46:43Z

Waiting for approval

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

the author
2 peers

Peer approvals must include a mandatory approval from @jonathan-gramain.

bert-e · 2020-06-24T23:59:51Z

History mismatch

Merge commit #c433d758b418f35bbf6ec63e143d670d8d9835ae on the integration branch
w/7.7/bugfix/S3C-2987-helperForJsonStreamParsing is merging a branch which is neither the current
branch bugfix/S3C-2987-helperForJsonStreamParsing nor the development branch
development/7.7.

It is likely due to a rebase of the branch bugfix/S3C-2987-helperForJsonStreamParsing and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

jonathan-gramain · 2020-06-25T00:00:44Z

@bert-e reset

bert-e · 2020-06-25T00:00:54Z

Reset complete

I have successfully deleted this pull request's integration branches.

bert-e · 2020-06-25T00:01:05Z

Conflict

A conflict has been raised during the creation of
integration branch w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing with contents from w/7.8/bugfix/S3C-2987-helperForJsonStreamParsing
and development/8.1.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing origin/development/8.1
 $ git merge origin/w/7.8/bugfix/S3C-2987-helperForJsonStreamParsing
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing

Add a new helper function to get data from a stream as a JSON payload, optionally validated against a joi schema. Note: uses async/await, so updated the scality/Guidelines dependency to please the linter

bert-e · 2020-06-25T00:09:55Z

History mismatch

Merge commit #aaa91b659a547bdf187e90f34dc26ca5f4c149f3 on the integration branch
w/7.7/bugfix/S3C-2987-helperForJsonStreamParsing is merging a branch which is neither the current
branch bugfix/S3C-2987-helperForJsonStreamParsing nor the development branch
development/7.7.

It is likely due to a rebase of the branch bugfix/S3C-2987-helperForJsonStreamParsing and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

jonathan-gramain · 2020-06-25T00:10:03Z

@bert-e reset

bert-e · 2020-06-25T00:10:10Z

Reset complete

I have successfully deleted this pull request's integration branches.

bert-e · 2020-06-25T00:10:19Z

Conflict

A conflict has been raised during the creation of
integration branch w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing with contents from w/7.8/bugfix/S3C-2987-helperForJsonStreamParsing
and development/8.1.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing origin/development/8.1
 $ git merge origin/w/7.8/bugfix/S3C-2987-helperForJsonStreamParsing
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing

miniscruff · 2020-06-25T00:12:22Z

One thing I suggest is instead of utils holding a couple of stream methods, export each method directly with its own file.
Instead of
- lib/stream/utils.js
module.exports = {
    readJSONStreamObject,
};
use
+lib/stream/readJSONStreamObject.js
module.exports = readJSONStreamObject;
It's easy to discover and use the methods this way.

An alternative is a flat structure used by standard packages like python, go, or node. Most imports require no more than one path.
So something like:
const { streamToObj } = require('stream')

Even if the files are split up using index.js to manipulate and simplify the imports can be convenient. Very different from super nested structures from Java and C++ ( I think ).

bert-e · 2020-06-25T00:41:17Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge bugfix/S3C-2987-helperForJsonStreamParsing into
development/7.4
w/7.7/bugfix/S3C-2987-helperForJsonStreamParsing will be merged into development/7.7 (pull request INTEGRATION [PR#1126 > development/7.7] bugfix: S3C-2987 helper to get stream data as a JSON payload #1130)
w/7.8/bugfix/S3C-2987-helperForJsonStreamParsing will be merged into development/7.8 (pull request INTEGRATION [PR#1126 > development/7.8] bugfix: S3C-2987 helper to get stream data as a JSON payload #1131)
w/8.1/bugfix/S3C-2987-helperForJsonStreamParsing will be merged into development/8.1 (pull request INTEGRATION [PR#1126 > development/8.1] bugfix: S3C-2987 helper to get stream data as a JSON payload #1132)

The following branches will NOT be impacted:

development/6.4
stabilization/7.4.6

Follow integration pull requests if you would like to be notified of
build statuses by email.

bert-e · 2020-06-25T00:41:18Z

Waiting for approval

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

the author
2 peers

Peer approvals must include a mandatory approval from @jonathan-gramain.

jonathan-gramain · 2020-06-25T00:51:53Z

@bert-e approve

bert-e · 2020-06-25T00:52:05Z

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/7.4
✔️ development/7.7
✔️ development/7.8
✔️ development/8.1

The following branches will NOT be impacted:

development/6.4
stabilization/7.4.6

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 the admin team 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 the admin team now.

The following options are set: approve

bert-e · 2020-06-25T00:52:12Z

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

✔️ development/7.4
✔️ development/7.7
✔️ development/7.8
✔️ development/8.1

The following branches have NOT changed:

development/6.4
stabilization/7.4.6

Please check the status of the associated issue S3C-2987.

Goodbye jonathan-gramain.

miniscruff approved these changes Jun 24, 2020

View reviewed changes

rahulreddy reviewed Jun 24, 2020

View reviewed changes

naren-rajendran approved these changes Jun 24, 2020

View reviewed changes

jonathan-gramain force-pushed the bugfix/S3C-2987-helperForJsonStreamParsing branch from c433d75 to aaa91b6 Compare June 24, 2020 23:59

bugfix: S3C-2987 helper to get stream data as a JSON payload

3d064b9

Add a new helper function to get data from a stream as a JSON payload, optionally validated against a joi schema. Note: uses async/await, so updated the scality/Guidelines dependency to please the linter

jonathan-gramain force-pushed the bugfix/S3C-2987-helperForJsonStreamParsing branch from aaa91b6 to 3d064b9 Compare June 25, 2020 00:09

bert-e merged commit 3d064b9 into development/7.4 Jun 25, 2020

bert-e deleted the bugfix/S3C-2987-helperForJsonStreamParsing branch June 25, 2020 00:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bugfix: S3C-2987 helper to get stream data as a JSON payload #1126

bugfix: S3C-2987 helper to get stream data as a JSON payload #1126

jonathan-gramain commented Jun 24, 2020

bert-e commented Jun 24, 2020

bert-e commented Jun 24, 2020

miniscruff Jun 24, 2020

rahulreddy left a comment

bert-e commented Jun 24, 2020

bert-e commented Jun 24, 2020

bert-e commented Jun 24, 2020

jonathan-gramain commented Jun 25, 2020

bert-e commented Jun 25, 2020

bert-e commented Jun 25, 2020

bert-e commented Jun 25, 2020

jonathan-gramain commented Jun 25, 2020

bert-e commented Jun 25, 2020

bert-e commented Jun 25, 2020

miniscruff commented Jun 25, 2020

bert-e commented Jun 25, 2020

bert-e commented Jun 25, 2020

jonathan-gramain commented Jun 25, 2020

bert-e commented Jun 25, 2020

bert-e commented Jun 25, 2020

bugfix: S3C-2987 helper to get stream data as a JSON payload #1126

bugfix: S3C-2987 helper to get stream data as a JSON payload #1126

Conversation

jonathan-gramain commented Jun 24, 2020

bert-e commented Jun 24, 2020

Hello jonathan-gramain,

bert-e commented Jun 24, 2020

Conflict

miniscruff Jun 24, 2020

Choose a reason for hiding this comment

rahulreddy left a comment

Choose a reason for hiding this comment

bert-e commented Jun 24, 2020

Integration data created

bert-e commented Jun 24, 2020

Waiting for approval

bert-e commented Jun 24, 2020

History mismatch

jonathan-gramain commented Jun 25, 2020

bert-e commented Jun 25, 2020

Reset complete

bert-e commented Jun 25, 2020

Conflict

bert-e commented Jun 25, 2020

History mismatch

jonathan-gramain commented Jun 25, 2020

bert-e commented Jun 25, 2020

Reset complete

bert-e commented Jun 25, 2020

Conflict

miniscruff commented Jun 25, 2020

bert-e commented Jun 25, 2020

Integration data created

bert-e commented Jun 25, 2020

Waiting for approval

jonathan-gramain commented Jun 25, 2020

bert-e commented Jun 25, 2020

In the queue

bert-e commented Jun 25, 2020