Sage-Bionetworks / synapsePythonClient

Programmatic interface to Synapse services for Python
https://www.synapse.org
Apache License 2.0
67 stars 68 forks source link

[SYNPY-1302] Replace getPermission with get_acl and add new get_permissions #1037

Closed danlu1 closed 9 months ago

danlu1 commented 9 months ago

Problem:

  1. The original getPermission method uses /acl endpoint only that shows inconsistent behavior in the case of publicly downloadable content from Synapse.

Solution:

  1. create a get_permissions method that implements GET /entity/{id}/permissions API. These API factors in the entity's ACL and the user's group membership when determining the caller's permission to the entity.
  2. rename getPermission with get_acl indicating it extracts ACL information only
  3. Deprecate getPermission

Example:

Screenshot 2023-12-15 at 9 31 28 AM Screenshot 2023-12-26 at 2 56 46 PM

Testing: Unit tests and integration tests are included in the PR.

pep8speaks commented 9 months ago

Hello @danlu1! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 2:89: E501 line too long (90 > 88 characters) Line 12:89: E501 line too long (89 > 88 characters) Line 22:89: E501 line too long (110 > 88 characters) Line 23:89: E501 line too long (114 > 88 characters) Line 25:89: E501 line too long (97 > 88 characters) Line 29:89: E501 line too long (106 > 88 characters) Line 30:89: E501 line too long (118 > 88 characters) Line 32:89: E501 line too long (113 > 88 characters) Line 34:89: E501 line too long (112 > 88 characters) Line 38:89: E501 line too long (119 > 88 characters) Line 39:89: E501 line too long (119 > 88 characters) Line 59:89: E501 line too long (103 > 88 characters) Line 74:89: E501 line too long (93 > 88 characters) Line 81:89: E501 line too long (118 > 88 characters) Line 84:89: E501 line too long (93 > 88 characters) Line 90:89: E501 line too long (111 > 88 characters) Line 93:89: E501 line too long (93 > 88 characters) Line 138:89: E501 line too long (96 > 88 characters) Line 155:89: E501 line too long (118 > 88 characters)

Line 620:89: E501 line too long (98 > 88 characters) Line 647:89: E501 line too long (96 > 88 characters) Line 713:89: E501 line too long (106 > 88 characters) Line 797:89: E501 line too long (110 > 88 characters) Line 884:89: E501 line too long (90 > 88 characters) Line 910:89: E501 line too long (114 > 88 characters) Line 933:89: E501 line too long (90 > 88 characters) Line 934:89: E501 line too long (93 > 88 characters) Line 940:89: E501 line too long (112 > 88 characters) Line 959:89: E501 line too long (95 > 88 characters) Line 963:89: E501 line too long (123 > 88 characters) Line 967:89: E501 line too long (103 > 88 characters) Line 1005:89: E501 line too long (122 > 88 characters) Line 1024:89: E501 line too long (99 > 88 characters) Line 1033:89: E501 line too long (99 > 88 characters) Line 1037:89: E501 line too long (123 > 88 characters) Line 1042:89: E501 line too long (105 > 88 characters) Line 1057:89: E501 line too long (92 > 88 characters) Line 1088:89: E501 line too long (115 > 88 characters)

Comment last updated at 2023-12-29 16:30:09 UTC
BryanFauble commented 9 months ago

Excellent work! Thank you for incorporating the handful of suggested changes. There are a few more I added around documentation and some code smells that SonarCloud flagged.

sonarcloud[bot] commented 9 months ago

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

7 New issues
0 Security Hotspots
100.0% Coverage on New Code
13.4% Duplication on New Code

See analysis details on SonarCloud