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

Use varying access level on version/runs methods and add tests #13

Merged
merged 11 commits into from Jan 20, 2021

Conversation

Xarthisius
Copy link
Contributor

@Xarthisius Xarthisius commented Jan 11, 2021

This PR adds and enables tests along with other project maintenance related boilerplate. I sneaked in one minor change related to how ACLs are handled. Before most of the models were forcefully loaded and a single routine _checkAccess was called to see if a user has write access to it. Instead of doing that we now load individual objects with proper and varied access level, which is a first step towards integration with our Tale sharing functionality.

How to test?

There's not much to test here really. Optionally follow testing steps from whole-tale/ngx-dashboard#55 and see it still works

@Xarthisius Xarthisius force-pushed the updates branch 2 times, most recently from 204e3f0 to 24c8e55 Compare January 11, 2021 17:11
@codecov
Copy link

codecov bot commented Jan 11, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@1e5dbb5). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #13   +/-   ##
=========================================
  Coverage          ?   73.00%           
=========================================
  Files             ?        6           
  Lines             ?      515           
  Branches          ?        0           
=========================================
  Hits              ?      376           
  Misses            ?      139           
  Partials          ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e5dbb5...23ae026. Read the comment docs.

@Xarthisius Xarthisius changed the title Updates Use varying access level on version/runs methods and add tests Jan 11, 2021
@craig-willis
Copy link
Contributor

craig-willis commented Jan 12, 2021

Unrelated to this PR, but while testing it I ran into the following error:

{"message": "AttributeError: AttributeError(\"'Version' object has no attribute '_isSame'\")", "trace": [
"<FrameSummary file /girder/girder/api/rest.py, line 630 in endpointDecorator>", 
"<FrameSummary file /girder/girder/api/rest.py, line 1227 in POST>", 
"<FrameSummary file /girder/girder/api/rest.py, line 970 in handleRoute>", 
"<FrameSummary file /girder/girder/api/access.py, line 63 in wrapped>", 
"<FrameSummary file /girder/girder/api/rest.py, line 445 in wrapped>", 
"<FrameSummary file /girder/girder/api/describe.py, line 709 in wrapped>", 
"<FrameSummary file /girder/plugins/wt_versioning/server/resources/version.py, line 118 in create>", 
"<FrameSummary file /girder/plugins/wt_versioning/server/resources/version.py, line 241 in _create>",
"<FrameSummary file /girder/plugins/wt_versioning/server/resources/version.py, line 289 in snapshot>", 
"<FrameSummary file /girder/plugins/wt_versioning/server/resources/version.py, line 362 in _sameTree>"], 
"type": "internal"}

How it happened:

@Xarthisius
Copy link
Contributor Author

Unrelated to this PR, but while testing it I ran into the following error:

How it happened:

  • Using Tale imported from whole-tale/girderfs#20 (contains a directory)
  • Create version v1 with Force checked
  • Create version v2 with Force unchecked

Looking at this method it seems it was meant to be recursive, so _isSame -> _sameTree should fix it, but I'll let @hategan to chime in.

@hategan
Copy link
Collaborator

hategan commented Jan 12, 2021

Unrelated to this PR, but while testing it I ran into the following error:
How it happened:

  • Using Tale imported from whole-tale/girderfs#20 (contains a directory)
  • Create version v1 with Force checked
  • Create version v2 with Force unchecked

Looking at this method it seems it was meant to be recursive, so _isSame -> _sameTree should fix it, but I'll let @hategan to chime in.

Hmm, looks like I've never tested with directories in the workspace. Yes, it should be a recursive call.

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

3 participants