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

feat(public-files): check authz/acl field, then hit arborist as anon … #653

Merged
merged 8 commits into from
Jun 28, 2019

Conversation

Avantol13
Copy link
Contributor

New Features

Breaking Changes

Bug Fixes

Improvements

  • fence will now consider authz field on indexd record to determine if a file is public or not (for different signed url behavior). previously only checked acl field

Dependency updates

Deployment changes

@PlanXCyborg
Copy link
Contributor

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

@coveralls
Copy link

coveralls commented Jun 27, 2019

Pull Request Test Coverage Report for Build 7147

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.004%) to 68.135%

Totals Coverage Status
Change from base Build 7096: -0.004%
Covered Lines: 4935
Relevant Lines: 7243

💛 - Coveralls

@@ -365,7 +367,7 @@ def metadata(self):

@cached_property
def public(self):
return check_public(self.set_acls)
return check_public(self.authz)
Copy link
Contributor

Choose a reason for hiding this comment

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

a couple thoughts:

  • check_public only used here so this would be a good opportunity to inline
  • should keep the ACLs check here too I think—we can use authz if exists or default to acl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, this is perhaps misleading, the cached property isn't just authz, it'll get acl field if authz doesn't exist. I will inline check_public tho

@Avantol13 Avantol13 merged commit ee6283c into master Jun 28, 2019
@Avantol13 Avantol13 deleted the fix/public-files branch June 28, 2019 18:27
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.

None yet

4 participants