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

fix #1987 #2047

Merged
merged 1 commit into from
Jan 17, 2017
Merged

fix #1987 #2047

merged 1 commit into from
Jan 17, 2017

Conversation

antgonza
Copy link
Member

No description provided.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 91.51% when pulling 73a78e7 on antgonza:fix-1987 into 80c5fea on biocore:master.

from qiita_db.study import Study


class ForRecursion(object):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you are facing the same issue that I was facing when doing the analysis refactor. Try changing this:

https://github.com/biocore/qiita/pull/2040/files#diff-694f6ed1529987bc3435746a74426405R408

then the functions should work

@antgonza
Copy link
Member Author

Perhaps but I think the current method works fine, right?

@josenavas
Copy link
Contributor

I was looking at the code and I'm not sure if we really want to go and modify the visibility of the current artifacts. I think it is ok for moving forward inherit the lowest visibility of the parents, but I don't think I agree with the python patch.

Basically, it is possible that the raw data is private, but the final BIOM is public, and it has been used in an analysis. According the current code, the BIOM would become private, and people that had access before to that BIOM and did analysis with it, no longer have access. I would not modify the current status of the artifacts in the DB.

@antgonza
Copy link
Member Author

Good point but I don't think that case is actually possible, right? See: https://github.com/biocore/qiita/blob/master/qiita_db/artifact.py#L621. I could change it to check if any children in the "family" is public/private and if it is make them all follow that visibility.

Copy link
Contributor

@tanaes tanaes left a comment

Choose a reason for hiding this comment

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

Maybe one typo (?) but not blocking for a patch ...

a.visibility = status
except:
# print so we know which changes failed and we can deal by hand
print "failed aid: %d, status %s" % (artifact.id, status)
Copy link
Contributor

Choose a reason for hiding this comment

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

Failed "aid?" Not sure I would understand what that was saying if I got this message.

@josenavas josenavas merged commit 0dcae8b into qiita-spots:master Jan 17, 2017
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