Skip to content

Conversation

@ssalaues
Copy link

This allows for replica sets to be defined and used as the metadata backends

config.json Outdated
"database": "metadata"
},
"log": {
"logLevel": "trace"
Copy link
Contributor

Choose a reason for hiding this comment

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

it should not be trace

MongoClient.connect(mongoUrl, (err, client) => {
if (err) {
throw (errors.InternalError);
throw (err);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not good here: We should log.error(err.message) (or similar field) and return errors.InternalError

'attributes to metastore',
{ error: err });
throw (errors.InternalError);
throw (err);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for all errors ...

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@ssalaues ssalaues force-pushed the ft/kub-mongo branch 5 times, most recently from 1e45272 to 250283c Compare January 30, 2018 22:02
// initialize this backend
MongoClient.connect(mongoUrl, (err, client) => {
if (err) {
this.logger.error('error connecting to mongo server', err);

Choose a reason for hiding this comment

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

For werelogs logging, the pattern is to have the second argument as an object containing additional information. For example, ...mongo server', { error: err });. I guess I would assume we should maintain that for these changes as well?

@ssalaues ssalaues force-pushed the ft/kub-mongo branch 3 times, most recently from dfbb3ac to 1848025 Compare January 31, 2018 18:20
@bennettbuchanan
Copy link

@ironman-machine r+

@ironman-machine
Copy link
Contributor

Hello @bennettbuchanan

"r+": Success

@ironman-machine
Copy link
Contributor

There is already a pending pull request on the same branch

@ironman-machine
Copy link
Contributor

⌛ PR is now pending. CI build url: http://ci.ironmann.io/gh/scality/Integration/18941

@ssalaues
Copy link
Author

ssalaues commented Jan 31, 2018

Didn't realize I had some linting issues. Fixed the issues and should be okay now.

@bennettbuchanan
Copy link

@ironman-machine r+

@ironman-machine
Copy link
Contributor

Hello @bennettbuchanan

"r+": Failure: Pull request isn't approved

@bennettbuchanan
Copy link

@ironman-machine r+

@ironman-machine
Copy link
Contributor

Hello @bennettbuchanan

"r+": Failure: Pull request isn't approved

@bennettbuchanan
Copy link

@ironman-machine try

@ironman-machine
Copy link
Contributor

Hello @bennettbuchanan

"try": Success: Try build successfully launched on 'http://ci.ironmann.io/gh/scality/Integration/18946' with the following env. args:

{
    "SCALITY_INTEGRATION_BRANCH": "ultron/master",
    "REPO_NAME": "S3",
    "DEFAULT_BRANCH": "master",
    "SCALITY_S3_BRANCH": "ft/kub-mongo"
}

@ironman-machine
Copy link
Contributor

☀️ 👍 circleCI test succeeded!

@ssalaues
Copy link
Author

Failing the awssdk tests. Will make the necessary changes.

upsert: true,
}, () => cb());
}, err => {
log.error('putObjectNoVer: error putting obect with no versioning',
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is causing error ""Cannot read property 'message' of null", needs an if (err) statement

@vrancurel
Copy link
Contributor

@ironman-machine r+

@ironman-machine
Copy link
Contributor

Hello @vrancurel

"r+": Failure: Pull request isn't approved

@LaurenSpiegel
Copy link
Contributor

@ironman-machine r+

@ironman-machine
Copy link
Contributor

Hello @LaurenSpiegel

"r+": Failure: Pull request isn't approved

@LaurenSpiegel
Copy link
Contributor

@ironman-machine r+

@ironman-machine
Copy link
Contributor

Hello @LaurenSpiegel

"r+": Success

@ironman-machine
Copy link
Contributor

There is already a pending pull request on the same branch

@LaurenSpiegel
Copy link
Contributor

@ironman-machine try

@ironman-machine
Copy link
Contributor

Hello @LaurenSpiegel

"try": Success: Try build successfully launched on 'http://ci.ironmann.io/gh/scality/Integration/18998' with the following env. args:

{
    "SCALITY_INTEGRATION_BRANCH": "ultron/master",
    "REPO_NAME": "S3",
    "DEFAULT_BRANCH": "master",
    "SCALITY_S3_BRANCH": "ft/kub-mongo"
}

@ironman-machine
Copy link
Contributor

⌛ PR is now pending. CI build url: http://ci.ironmann.io/gh/scality/Integration/19001

@ironman-machine
Copy link
Contributor

☀️ 👍 circleCI test succeeded!

@LaurenSpiegel LaurenSpiegel merged commit cd252ff into master Feb 1, 2018
@LaurenSpiegel LaurenSpiegel deleted the ft/kub-mongo branch February 1, 2018 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants