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

[JENKINS-46929] Limit number of projects added to response headers #532

Merged
merged 2 commits into from
Sep 22, 2017

Conversation

dwnusbaum
Copy link
Member

@dwnusbaum dwnusbaum commented Sep 22, 2017

JENKINS-46929

I changed GitStatus#doNotifyCommit to only add up to 10 "Triggered" headers (which contain URLs to downstream projects for which polling or builds have been scheduled by this commit) to its response. This should prevent java.io.IOException: Response header too large exceptions when a repository is used in a large number of projects.

@reviewbybees

@@ -609,6 +614,7 @@ public String getShortDescription() {
}

private static final Logger LOGGER = Logger.getLogger(GitStatus.class.getName());
private static final int MAX_REPORTED_CONTRIBUTORS = 10;
Copy link
Member Author

Choose a reason for hiding this comment

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

I made the limit 10 arbitrarily. Does anyone have a strong opinion on a different limit? Also, does anyone see value in allowing the limit to be set via environment variable, so that anything relying on the urls in the headers could set it as a workaround?

@ghost
Copy link

ghost commented Sep 22, 2017

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'd leave the property internal to the class until there are multiple requests for it. That particular header doesn't seem very useful to me.

@MarkEWaite MarkEWaite merged commit 35745f2 into jenkinsci:master Sep 22, 2017
@dwnusbaum dwnusbaum deleted the JENKINS-46929 branch September 28, 2017 13:54
@MarkEWaite MarkEWaite added the bugfix Fixes a bug - used by Release Drafter label Jul 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a bug - used by Release Drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants