-
Notifications
You must be signed in to change notification settings - Fork 26
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
notice when app engine java components aren't installed #206
Conversation
@@ -59,6 +59,7 @@ | |||
private static final String DEV_APPSERVER_PY = "bin/dev_appserver.py"; | |||
private static final String JAVA_APPENGINE_SDK_PATH = | |||
"platform/google_appengine/google/appengine/tools/java/lib"; | |||
private static final String JAVA_COMPONENTS_PATH = "platform/gcd/.appengine"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it sufficient to just check for the existence of JAVA_APPENGINE_SDK_PATH
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that seems to exist regardless
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably check with the Cloud SDK team to see what they recommend using.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would executing and parsing a gcloud components list
be overkill?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markpell, @znewman01 can you comment on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to know whether or not the user ran:
gcloud components install app-engine-java
We've seen multiple user reports that trace back to not having done this.
This directory appears to be added/removed when the user installs/removes the component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW: although my installation is out of date, I did have just updated and I don't have anything in app-engine-java
, butplatform/gcd
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since what we're actually using is the appengine-tools-api.jar, we should probably just check for its existence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So again, this is just checking for the wrong thing. That directory is for the legacy datastore emulator which has nothing to do with Java or any Java components.
How far are we from having the requiring staging code as separate artifacts on maven central? I don't think it's appropriate for the plugin to be grabbing individual jars from the SDK as this is not a public interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ludoch who would know when staging is going to make it to maven central.
To summarize what's being discussed:
For component detection we should be calling gcloud components list and not digging into the cloud SDK packaged files.
The staging question is not related to this CL but we are working on moving as many deps as possible away from using Cloud SDK paths.
$ gcloud components list --format='value(state)' --filter=id:app-engine-java Your current Cloud SDK version is: 124.0.0 name=Not Installed |
Hmm, I should update. |
I think you'd want to set |
@@ -262,6 +268,10 @@ public void validate() throws AppEngineException { | |||
"Validation Error: Java Tools jar location '" | |||
+ JAR_LOCATIONS.get(JAVA_TOOLS_JAR) + "' is not a file."); | |||
} | |||
if (!Files.isDirectory(getJavaAppEngineComponentsPath())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my concern about having this here is that we can support deploy without needing the java components installed. A client should be able to get configure gcloud and do a deployment without install the components,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, this only applies to Standard and Compat deployments, not pure Flex.
$ gcloud components list --format='json' --filter=id:app-engine-java Your current Cloud SDK version is: 124.0.0 [ It really shouldn't be saying Your current Cloud SDK version is: 124.0.0 |
It's only printing that to STDERR; it's very easy to ignore. On Wed, Sep 7, 2016 at 6:35 PM, Elliotte Rusty Harold <
|
@elharo are we closing this to start over? |
Yes, the version that calls gcloud components shares very little with this branch. |
reopening since we may do this |
@meltsufin @etanshaul fix #165