Skip to content

Feature/provisioning api #176

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

Merged
merged 8 commits into from
Aug 8, 2019
Merged

Feature/provisioning api #176

merged 8 commits into from
Aug 8, 2019

Conversation

nitzanj
Copy link
Contributor

@nitzanj nitzanj commented Aug 7, 2019

No description provided.

@nitzanj nitzanj requested a review from asisayag2 August 7, 2019 07:43
private ApiResponse callAccountApi(Api.HttpMethod method, List<String> uri, Map<String, Object> params, Map<String, Object> options) throws Exception {
options = verifyOptions(options);

if (!options.containsKey("provisioning_api_key")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@nitzanj , you assume that if someone is sending the key in the options, he's also sending the secret?
Let's verify that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't mix and match, it's probably better to end up having just the key if the secret is missing, otherwise we end up in an inconsistent state. Let's discuss it tomorrow to go over all the scenarios...

Copy link
Contributor

Choose a reason for hiding this comment

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

I know we can't. that's why I suggest verify that both were sent and react if not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But then if we support key only API, there's no way to override the config secret (e.g. send null in the options, which will only be used if the key is there).

If we can safely assume we don't have key only APIs (only uploader has some of those) then we should indeed verify both are in the options.

return callAccountApi(Api.HttpMethod.GET, uri, Collections.<String, Object>emptyMap(), options);
}

public ApiResponse getSubAccounts(Boolean enabled, List<String> ids, String prefix, Map<String, Object> options) throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

@nitzanj ,what's 'enabled' used for?
Please add JavaDoc exaplining

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There will be full javadoc before merging (working on it already).

"name", name,
"custom_attributes", customAttributes,
"enabled", enabled,
"from_base_account", baseAccount),
Copy link
Contributor

Choose a reason for hiding this comment

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

@nitzanj , spec calls this "base_sub_account_id"

@nitzanj nitzanj merged commit 63bc21a into master Aug 8, 2019
@nitzanj nitzanj deleted the feature/provisioning-api branch August 8, 2019 09: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.

2 participants