-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Another batch of SA2.0 edits in TS2.0 (part 3) #16833
Conversation
7efa868
to
1d46c5c
Compare
Also, add case_sensitive parameter to shared function in galaxy user manager
1d46c5c
to
bf919c0
Compare
0bad44e
to
68831dd
Compare
@@ -1107,3 +1066,39 @@ def _get_changeset_revisions_that_contain_tools(app: "ToolShedApp", repo, reposi | |||
if metadata.get("tools", None): | |||
changeset_revisions_that_contain_tools.append(changeset_revision) | |||
return changeset_revisions_that_contain_tools | |||
|
|||
|
|||
def get_user_by_username(session, username, user_model): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the user_model here the third and below it always the second argument?
Does the model really need to be passed in? Can we not assume it has the user model by the function name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. I passed all those models to these functions because in a different module directly importing them from tool_shed.webapp.model
raised an error due to a circular dependency. However, those were modules in util
, but this is in managers
, so there should be no error from an import. Let me clean this up a bit (I hate passing those models).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not reusing the function from galaxy.managers.users
because that one calls first()
, whereas this one calls one()
, so the behavior is slightly different. Most likely the difference is inconsequential, but with the tool shed I cannot be sure. And the calling code would have to be slightly adjusted. Given the scale of these changes, I'm trying to minimize my footprint here.
RepositoryMetadata is mapped imperatively, so until the mapper creates attributes based on Column definitions, the attribute does not exist on the class. This class is mapped imperatively due to the complications triggered by mapping the "metadata" attribute.
This is ready for review. |
SQLAlchemy 2.0 compatibility. Follow up to #16799. Ref #12541.
How to test the changes?
License