Update to core 0.6.6, project -> projectId #286
Conversation
|
PR coming up in appengine-plugins-core to read gcloud config that should let the user get data from gcloud state. |
| import org.xml.sax.SAXException; | ||
|
|
||
|
|
||
| public final class ConfigReader { |
There was a problem hiding this comment.
Change is here: extract out config reader for easier sharing.
|
|
||
| // configure the runExtension's project parameter | ||
| // assign the run project to the deploy project if none is specified | ||
| if (Strings.isNullOrEmpty(runExtension.getProjectId())) { |
There was a problem hiding this comment.
Change here: run's new paramter "projectId" is processed at configuration time.
| + GCLOUD_CONFIG | ||
| + "' to use project from gcloud config.\n" | ||
| + "3. Using " | ||
| + APPENGINE_CONFIG | ||
| + " is not allowed for flexible environment projects"); | ||
| } else if (configString.equals(GCLOUD_CONFIG)) { | ||
| return null; | ||
| return ConfigReader.from(gcloud).getProject(); |
There was a problem hiding this comment.
Change here: now reads in the config instead of delegating to gcloud.
| deploy.setDeployTargetResolver(new StandardDeployTargetResolver(appengineWebXml)); | ||
|
|
||
| // configure the deploy extensions's project/version parameters | ||
| StandardDeployTargetResolver resolver = |
There was a problem hiding this comment.
Change here: deploy now computers at configuration time the projectId and version.
| } else if (configString.equals(GCLOUD_CONFIG)) { | ||
| return null; | ||
| return ConfigReader.from(gcloud).getProject(); |
There was a problem hiding this comment.
Change here: read gcloud project id at config time.
| } | ||
|
|
||
| /** Return "application" tag from appengine-web.xml or error out if could not read. */ | ||
| public String getProject() { |
There was a problem hiding this comment.
nit: could this be "combined" with getVersion?
There was a problem hiding this comment.
I was trying to contain all exceptions and exception handling in the methods themselves. But I guess I could potentially move the parsing to the constructor and still have almost the same effect.
There was a problem hiding this comment.
it turns out the call to getProjectId is still doing some Dom Processing, so I don't think we get too much from refactoring out initializing the appengineDescriptor
There was a problem hiding this comment.
Or were you saying just returning project+version from one method?
There was a problem hiding this comment.
Oh yea, I was just referring to refactoring them together, but since it doesn't same much, this is good.
|
|
||
| DeployExtension deploy = | ||
| p.getExtensions().getByType(AppEngineStandardExtension.class).getDeploy(); | ||
| deploy.setProjectId("project"); |
There was a problem hiding this comment.
Are these sets necessary? (I see the same being set on the deploy extension in the test build.gradle files)
There was a problem hiding this comment.
yeah, this programmatic creation of the project isn't using a build file. And since we're resolving project/version a little earlier in the build, it will fail at configuration time if they're not set.
USER_GUIDE.md
Outdated
| Caution: The v2-alpha version of the development web server is not fully | ||
| supported, and you may find errors when using this version. | ||
|
|
||
| To switch Dev App Server v2-alpha use the `serverVersion` parameter. |
| @@ -373,7 +377,7 @@ The `deploy` configuration has the following parameters : | |||
| | `appEngineDirectory` | Location of configuration files (cron.yaml, dos.yaml, etc) for configuration specific deployments. | | |||
| | `bucket` | The Google Cloud Storage bucket used to stage files associated with the deployment. | | |||
| | `imageUrl` | Deploy with a Docker URL from the Google container registry. | | |||
| | `project` | The Google Cloud Project target for this deployment. This can also be set to `GCLOUD_CONFIG`.\* | | |||
| | `projectId` | The Google Cloud Project target for this deployment. This can also be set to `GCLOUD_CONFIG`.\* | | |||
There was a problem hiding this comment.
Could this one also be APPENGINE_CONFIG?
There was a problem hiding this comment.
flex doesn't allow appengine-web.xml based config. So they can't use APPENGINE_CONFIG here.
| } | ||
|
|
||
| /** Return "application" tag from appengine-web.xml or error out if could not read. */ | ||
| public String getProject() { |
There was a problem hiding this comment.
Hmm, these could just be static methods?
ConfigReader#getProject(File appengineWebXml)
ConfigReader#getVersion(File appengineWebXml)
ConfigReader#getProject(Gcloud gcloud)
There was a problem hiding this comment.
yeah, good point.
fixes #284