-
Bug
-
Resolution: Done
-
Major
-
4.5.3.AM3
-
None
-
devex #147 April 2018
This is a followup of JBIDE-25835 .
Recently a new feature was added - cdk can be downloaded directly from the IDE now.
As with other runtimes, there are two main paths you can take:
a) New Server -> Download
b) Preferences -> Runtime Detection -> Download
tl;dr: b) doesn't work properly with cdk because minishift binary is still not set after the download.
There is quite a big difference between a) and b). With a), you are adding a server manually and once your download is finished, the correct path is predefined for you. Runtime detection is not involved.
With b), the server is downloaded, extracted and then the path is added to runtime detection which is in turn run. This will result in the new server being detected and added.
Now cdk should theoretically support both of these paths. a) works - you're in the middle of adding a cdk server manually, you invoke the download and once that's done, the minishift binary field is filled properly for you.
But b) doesn't work properly for cdk, because runtime detection for cdk is built around your minishift home and not the minishift binary path. (I don't really know why that's the case, but I guess you had a good reason for it.) So when you do b), cdk binary is downloaded, but what's added to runtime detection is ~/.minishift (or $MINISHIFT_HOME if set). The result is that the cdk server adapter is added once the download is finished, but the minishift binary remains empty. So the usefulness of this feature is questionable.
Maybe you will say that there is no way to fix this. I hope you will at least agree that this is not ideal and b) doesn't really work properly right now.
I don't know what the proper solution for this is but let me start with a question: Wouldn't it make sense to change runtime detection of cdk so that it uses the minishift binary as the main path? What's stopping us from doing that?
Sorry for the lengthy description, I felt I needed to explain the whole context here.
- relates to
-
-
- Closed
-
-
-
- Closed
-
-
-
- Closed
-
[JBIDE-25858] Minishift binary is not set up after CDK runtime download
Martin Malina
added a comment - For the record, I did click on the link 
But didn't really read the code - just checked what was that commit. I admit I could have at least briefly read the code.
Martin Malina
added a comment - For the record, I did click on the link But didn't really read the code - just checked what was that commit. I admit I could have at least briefly read the code. 
Rob Stryker (Inactive)
added a comment - This is fixed as far as I can tell Rob Stryker (Inactive)
added a comment - If you clicked on teh code link I sent, you'd see it does both.
+ if( isValidMinishiftHome(root)) { + return createHomeDefinition(root); + } + + if( folderContainsMinishiftBinary(root)) { + return createBinaryDefinition(root); + } + return null; + }
Rob Stryker (Inactive)
added a comment - If you clicked on teh code link I sent, you'd see it does both.
+ if ( isValidMinishiftHome(root)) {
+ return createHomeDefinition(root);
+ }
+
+ if ( folderContainsMinishiftBinary(root)) {
+ return createBinaryDefinition(root);
+ }
+ return null ;
+ }
Martin Malina
added a comment - rob.stryker, I just a tried the latest nightly of devstudio and I don't see any change. You're saying that runtime detection is now based on path to minishift binary and not minishift home. But I installed a new installation of devstudio, started it and it already has a cdk adapter. I checked runtime detection and it has the same path included as before - ~/.minishift. And the adapter has empty binary and minishift home is setup. How does this work?
I tried adding ~/tmp to runtime detection and it did find my cdk binary there, so year, that seems to work. (Although the binary path will be missing in the newly created adapter, but that seems to be a separate bug that you will fix.)
So the question is: How come there is still an adapter added when I start devstudio? It seems to me that runtime detection will now work both paths - either path to minishift home or path to search for minishift binary. Is that the case?
Martin Malina
added a comment - rob.stryker , I just a tried the latest nightly of devstudio and I don't see any change. You're saying that runtime detection is now based on path to minishift binary and not minishift home. But I installed a new installation of devstudio, started it and it already has a cdk adapter. I checked runtime detection and it has the same path included as before - ~/.minishift. And the adapter has empty binary and minishift home is setup. How does this work?
I tried adding ~/tmp to runtime detection and it did find my cdk binary there, so year, that seems to work. (Although the binary path will be missing in the newly created adapter, but that seems to be a separate bug that you will fix.)
So the question is: How come there is still an adapter added when I start devstudio? It seems to me that runtime detection will now work both paths - either path to minishift home or path to search for minishift binary. Is that the case? Martin Malina
added a comment - Good to hear about that change, I wasn't aware! Thanks! Rob Stryker (Inactive)
added a comment - The reason this doesn't seem to work is a simple and dumb coding error. It is not the case that minishift home is the base of runtime detection. This was changed on March 20th. See this commit: https://github.com/jbosstools/jbosstools-openshift/pull/1697/files#diff-b8a2722a5c9efb2fb551fd3ff0a47b75R59
However... this error was due to poor logic, unfortunately. PR to come shortly.
Rob Stryker (Inactive)
added a comment - The reason this doesn't seem to work is a simple and dumb coding error. It is not the case that minishift home is the base of runtime detection. This was changed on March 20th. See this commit: https://github.com/jbosstools/jbosstools-openshift/pull/1697/files#diff-b8a2722a5c9efb2fb551fd3ff0a47b75R59
However... this error was due to poor logic, unfortunately. PR to come shortly. 
It works as expected now (devstudio 11.3.0.GA), for the most part. Closing.