Skip to content

Dont attempt to set email address property for a user upon login#14

Merged
skottler merged 2 commits into
jenkinsci:masterfrom
suryagaddipati:master
Jan 23, 2014
Merged

Dont attempt to set email address property for a user upon login#14
skottler merged 2 commits into
jenkinsci:masterfrom
suryagaddipati:master

Conversation

@suryagaddipati
Copy link
Copy Markdown
Member

This causes performance issues when logging in.

if (u.getProperty(Mailer.UserProperty.class).getAddress() == null)

This ultimately invokes

AbstractProject.hasParticipant()

on each and every project.
We have hundreds of projects and this causes logging in to take forever ( more than 5 minutes) .

This causes performace issues when logging in.
@buildhive
Copy link
Copy Markdown

Jenkins » github-oauth-plugin #43 SUCCESS
This pull request looks good
(what's this?)

@jenkinsadmin
Copy link
Copy Markdown
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@suryagaddipati
Copy link
Copy Markdown
Member Author

can someone merge this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could probably just use hasExplicitlyConfiguredAddress() instead of getAddress() to skip the possibly expensive lookup while retaining the feature to set the email address automatically.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@suryagaddipati can you look a look at the comment @daniel-beck left above?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@daniel-beck @skottler i couldn't find hasExplicitlyConfiguredAddress() . Where is it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same Mailer.UserProperty as getAddress(). Source code reference

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@daniel-beck oh thanks. I've changed the check to use that. Seems to address the performance issue.

…all projects and builds to find users's email address)
@cloudbees-pull-request-builder
Copy link
Copy Markdown

plugins » github-oauth-plugin #15 SUCCESS
This pull request looks good

@suryagaddipati
Copy link
Copy Markdown
Member Author

Can someone merge this?

skottler pushed a commit that referenced this pull request Jan 23, 2014
Dont attempt to set email address property for a user upon login
@skottler skottler merged commit b4ae949 into jenkinsci:master Jan 23, 2014
@skottler
Copy link
Copy Markdown
Member

Sorry I haven't merged it earlier. Done.

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.

6 participants