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

Refactor startup check before running any watch #25

Merged
merged 1 commit into from
Nov 18, 2016
Merged

Refactor startup check before running any watch #25

merged 1 commit into from
Nov 18, 2016

Conversation

jimmidyson
Copy link
Contributor

No description provided.

try {
buildAdded(b);
} catch (IOException e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

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

change to log statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -114,6 +116,28 @@ private void configChange() {
OpenShiftUtils.initializeOpenShiftClient(server);
this.namespace = getNamespaceOrUseDefault(namespace, getOpenShiftClient());

logger.info("Waiting for Jenkins to be started");
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering why you need to do this here? Can you not wait until all jobs are loaded? Take a look at this: http://javadoc.jenkins.io/hudson/model/listeners/ItemListener.html#onLoaded()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. TBH I'm not 100% sure that everything is in the right state when ItemListener.onLoaded is called. The current way may not be the "best" or most correct way to do it but we know it works. I've raised openshift#109 to track it & test it out once I get some time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer to keep it the way it is right now & try out the better way you've suggested once time allows. Are you OK with that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me!

@jimmidyson jimmidyson merged commit 03aebdd into jenkinsci:master Nov 18, 2016
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.

2 participants