Skip to content
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.

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

ft: ZENKO-319 adds sproxyd location type management #1242

Merged

Conversation

alexanderchan-scality
Copy link
Contributor

No description provided.

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@@ -171,6 +171,20 @@ function patchConfiguration(instanceId, newConf, log, cb) {
};
}
break;
case 'location-scality-sproxyd-v1':
location.type = 'scality';
if (l.details && l.details.bootstrap && l.detaisl.path) {
Copy link
Contributor

Choose a reason for hiding this comment

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

typo, should be l.details.path

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

locations.details = {
connector: {
sproxyd: {
chordCos: l.details.chordCos || null,
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to add ARC support here as well, or not (@rachedbenmustapha ?)

Copy link
Contributor

Choose a reason for hiding this comment

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

@jonathan-gramain you're the sproxyd guru so correct me if I'm wrong :) We rely on already-deployed sproxyd and I believe ARC settings will be bound to the proxyPath we are clients of.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, since we're using the proxyPath to an existing sproxyd that's what matters. But then is the chord COS used at all, aren't we passing the object bucket/key in the path directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jonathan-gramain chord cos is passed to sproxydclient

@ironman-machine ironman-machine dismissed stale reviews from rachedbenmustapha and jonathan-gramain May 11, 2018 18:59

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

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@ironman-machine
Copy link
Contributor

CONFLICT (add/add): Merge conflict in tests/unit/api/bucketACLauth.js
CONFLICT (add/add): Merge conflict in package.json
CONFLICT (add/add): Merge conflict in lib/api/objectGet.js
CONFLICT (add/add): Merge conflict in lib/api/apiUtils/bucket/bucketCreation.js
CONFLICT (add/add): Merge conflict in lib/api/apiUtils/authorization/aclChecks.js

@alexanderchan-scality alexanderchan-scality changed the base branch from z/1.0 to development/8.0 June 5, 2018 19:00
@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@rahulreddy
Copy link
Collaborator

@alexanderchan-scality Can you make this BertE compatible?

@ironman-machine ironman-machine dismissed stale reviews from jonathan-gramain and rachedbenmustapha June 7, 2018 17:10

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

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@rachedbenmustapha rachedbenmustapha merged commit 99d6f6f into development/8.0 Jun 7, 2018
@rachedbenmustapha rachedbenmustapha deleted the ft/ZENKO-319-sproxydLocationType branch June 7, 2018 20:38
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.

5 participants