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

Add "No connection on bootup" option #168

Merged
merged 5 commits into from Aug 12, 2014

Conversation

Projects
None yet
4 participants
@rinrinne
Copy link
Member

commented Aug 4, 2014

This patch adds a feature that connection to Gerrit is not established on bootup.

In addition, pseudo mode is removed since it is now unnecessary.

rinrinne added some commits Aug 4, 2014

Deprecate pseudo mode
Previously, gerrit handler could not accept any events during
disconnected. So pseudo mode was added to be able to get events
in any connection status.

After implementation of GerritServer, handler can accept events
during disconnected without pseudo mode. So this patch removes it.
Add "No connection on bootup" option in server config
This patch adds a feature that connection to server is not established
on bootup.

Fix for JENKINS-24099

Task-Url: https://issues.jenkins-ci.org/browse/JENKINS-24099
@cloudbees-pull-request-builder

This comment has been minimized.

Copy link

commented Aug 4, 2014

plugins » gerrit-trigger-plugin #302 SUCCESS
This pull request looks good

@jenkinsadmin

This comment has been minimized.

Copy link
Member

commented Aug 4, 2014

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

assertSame(Result.SUCCESS, buildOne.getResult());
assertEquals(1, projectOne.getBuilds().size());
assertSame(gerritServerOneName, buildOne.getCause(GerritCause.class).getEvent().getProvider().getName());
assertEquals(true, gerritServerOne.isNoConnectionOnBootup());

This comment has been minimized.

Copy link
@rsandell

rsandell Aug 11, 2014

Member

Can you somehow also check that it actually hasn't tried to connect?

@@ -136,7 +136,9 @@
private static final int RESPONSE_INTERVAL_MS = 1000;
private static final int RESPONSE_TIMEOUT_S = 10;
private String name;
@Deprecated
private boolean pseudoMode;

This comment has been minimized.

Copy link
@rsandell

rsandell Aug 11, 2014

Member

pseudoMode should also be made transient

This comment has been minimized.

Copy link
@rinrinne

rinrinne Aug 12, 2014

Author Member

When added transient, assertion error happened in deserialize phase. But it seems there was something wrong in my environment. Now works fine.

OK, I will add it.

@@ -0,0 +1 @@
Check if this server connects to Gerrit on bootup.

This comment has been minimized.

Copy link
@rsandell

rsandell Aug 11, 2014

Member

Isn't it the other way around?

Ex: "Check if this server should not connect to Gerrit on bootup."

This comment has been minimized.

Copy link
@rinrinne

rinrinne Aug 12, 2014

Author Member

Sorry, I missed. will update.

@rsandell

This comment has been minimized.

Copy link
Member

commented Aug 11, 2014

I think I would be more comfortable with the word "startup" instead of "bootup".
For me bootup is when the hardware starts from a power down, while startup is more arbitrary and closer to when Jenkins itself starts.

@rinrinne

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2014

In fact, I was troubled with naming we should adopt. Also polarities of value. As a result, I had mixed them. Sorry for confusing you...

So I sorted out like below:

  • Use "startup"
  • Not connect on startup if checked
@cloudbees-pull-request-builder

This comment has been minimized.

Copy link

commented Aug 12, 2014

plugins » gerrit-trigger-plugin #305 SUCCESS
This pull request looks good

rsandell added a commit that referenced this pull request Aug 12, 2014

Merge pull request #168 from rinrinne/boot-on-startup
Add "No connection on bootup" option

@rsandell rsandell merged commit b49ce36 into jenkinsci:master Aug 12, 2014

@rinrinne rinrinne deleted the rinrinne:boot-on-startup branch Aug 12, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.