Skip to content

Conversation

@smouli
Copy link
Contributor

@smouli smouli commented Jul 25, 2018

No description provided.

@smouli smouli changed the title ensure that incoming request to set_bucket_policy() is of type str Ensure that incoming request to set_bucket_policy() is of type str Jul 25, 2018
@nitisht nitisht requested review from ebozduman and poornas July 25, 2018 08:50
Copy link
Contributor

@poornas poornas left a comment

Choose a reason for hiding this comment

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

@smouli, travis failures need to be fixed

{"function": "set_bucket_policy(bucket_name, policy)", "status": "FAIL", "name": "minio-py:test_set_bucket_policy_readonly", "args": {"bucket_name": "minio-py-test-15715cb4-91e4-4707-ac7d-e92b9ee49723"}, "error": "Traceback (most recent call last):\n  File \"tests/functional/tests.py\", line 1806, in main\n    test_set_bucket_policy_readonly(client, log_output)\n  File \"tests/functional/tests.py\", line 1527, in test_set_bucket_policy_readonly\n    raise Exception(err)\nException: policy can only be of type str\n", "duration": 160, "message": "policy can only be of type str"}

minio/api.py Outdated
query={"policy": ""})

def set_bucket_policy(self, bucket_name, policy):
def set_bucket_policy(self, bucket_name, policy):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the space added by mistake at the end of the line.

minio/helpers.py Outdated

return True

def is_valid_bucket_policy_name(policy):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Function name is_valid_bucket_policy_name is misleading, since we are not validating the name of the policy.
Change the function name something like is_valid_type_bucket_policy.

minio/helpers.py Outdated
Validates bucket policy
:param policy: S3 style Bucket policy.
:return: True if input is a valid policy structure.
Raise :exc: `TypeError` otherwise.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is :exc:?

minio/helpers.py Outdated
"""
Validates bucket policy
:param policy: S3 style Bucket policy.
:return: True if input is a valid policy structure.
Copy link
Collaborator

Choose a reason for hiding this comment

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

:return: True if input is a valid policy structure.
=>
:return: True if policy parameter is of a valid type, 'string'.

@ebozduman
Copy link
Collaborator

We also need to change the documentation and examples.

@smouli smouli force-pushed the bucketPolicyCheck branch from 81e9d51 to aab1683 Compare August 1, 2018 09:07
@smouli
Copy link
Contributor Author

smouli commented Aug 1, 2018

@poornas I fixed travis failures
@ebozduman I made changes

minio/api.py Outdated
Set bucket policy of given bucket name.
:param bucket_name: Bucket name.
:param policy: Access policy/ies in JSON format.
Copy link
Collaborator

Choose a reason for hiding this comment

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

JSON format => string format

:param bucket_name: Bucket name.
:param policy: Access policy/ies in JSON format.
"""
is_valid_bucket_type_policy(policy)
Copy link
Contributor

Choose a reason for hiding this comment

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

is_valid_bucket_policy_type would be better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what about is_valid_policy_type as @ebozduman suggested

@smouli smouli force-pushed the bucketPolicyCheck branch from aab1683 to 4d5fd71 Compare August 1, 2018 16:40

return True

def is_valid_bucket_type_policy(policy):
Copy link
Collaborator

Choose a reason for hiding this comment

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

is_valid_bucket_type_policy => is_valid_policy_type

Raise :exc:`TypeError` otherwise.
"""
if _is_py3:
string_types = str,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor comment:
string_types => string_type

@smouli smouli force-pushed the bucketPolicyCheck branch from 4d5fd71 to e64a3c2 Compare August 1, 2018 16:57
@smouli
Copy link
Contributor Author

smouli commented Aug 1, 2018

I made a mistake and have to close this PR. The new PR is #687

@smouli smouli closed this Aug 1, 2018
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.

4 participants