openshift / jenkins-plugin

Apache License 2.0
81 stars 50 forks source link

Added a "Test Connection" button to each build step #83

Closed oatmealraisin closed 8 years ago

oatmealraisin commented 8 years ago

These check for a basic "Server Ready" status on the given URL, skipping TLS authentification

I've abandoned the idea of having global "server profiles" for now, due to time.

@gabemontero Unfortunately, extending IOpenShiftPlugin requires our descriptors to implement coreLogic(), which I don't think is desirable. I'll reach out to you to discuss possibly splitting IOpenShiftPlugin into a few more classes, so that we can implement only what we need.

@gabemontero @bparees ptal?

gabemontero commented 8 years ago

@oatmealraisin actually, I think it is better to extend IOpenShiftPlugin and provide a no-op implementation for coreLogic(). That way, you leverage all the existing code for connecting via the httpGet method and we can support TLS vs. no TLS in the same way like we discussed yesterday.

An agreeable splitting up / refactoring IOpenShiftPlugin to avoid the coreLogic no-op would be the best of both worlds, but I'm fine with deferring on that to get this in this week.

So apologies, but please switch back to the other approach we discussed.

gabemontero commented 8 years ago

I think I'm OK with the abandoing server profile piece. Still thinking about whether the new cluster.jelly @jupierce recently added is the right spot for this. But we can circle back to this after you refactor around IOpenShiftPlugin. I would think it is reasonably straight forward to move the UI widget between jelly files.

oatmealraisin commented 8 years ago

@gabemontero I still had trouble implementing IOpenShiftPlugin in this interface, so instead I just implemented httpGet. ptal?

gabemontero commented 8 years ago

@oatmealraisin can we either: 1) create an interface that both IOpenShiftPlugin and IOpenShiftPluginDescriptor extend that just has httpGet as a default method that shares the actual code / avoids code duplication where ever possible 2) create a static class that the httpGet in both interfaces leverages (again share code, etc. as much as possible)

At first blush 1) is my first choice.

oatmealraisin commented 8 years ago

@gabemontero I've changed httpGet to a static method, and also changed its signature to take vars it would normally get through getters. ptal?

gabemontero commented 8 years ago

As an FYI, we'll wait until a future sprint / effort to see about further refactoring to move off of the static class to a lower level class/interface structure that just has httpGet. @oatmealraisin and I discussed and came to this offline.

oatmealraisin commented 8 years ago

@gabemontero I've added the class :) ptal?

oatmealraisin commented 8 years ago

@gabemontero I also made the key for the kube env var a final string in IOpenShiftPlugin, ptal?

gabemontero commented 8 years ago

IGTM

A thought regarding testing .... can you find out, or do you already know, the URL of the HTTP request generated when you hit the test connection button? If so, we should be able to add an extended test.

oatmealraisin commented 8 years ago

@gabemontero Looking into it