GoogleCloudPlatform / app-gradle-plugin

The library has moved to https://github.com/GoogleCloudPlatform/appengine-plugins/tree/main/app-gradle-plugin
Apache License 2.0
153 stars 40 forks source link

Is managed SDK still working ? #304

Closed martinbonnin closed 2 years ago

martinbonnin commented 5 years ago

I just setup the plugin on a linux host and got this exception:

Validation Error: SDK location '/home/martin/.cache/google-cloud-tools-java/managed-cloud-sdk/LATEST/google-cloud-sdk' is not a directory.

After some investigations, it turns out that it works well if I force the cloud-sdk-home in appengine.tools.cloudSdkHome.

I am still confused because the documentation says that CloudSDK needs to be downloaded before running the plugin but the plugin contains a tasks to download it as well.

Can you clarify how to setup the plugin ?

loosebazooka commented 5 years ago

Can you show me your config? (build.gralde)

martinbonnin commented 5 years ago

Config below. It's working now. To clarify, my issue is that it took me a couple of hours this morning to figure this stuff out. At some point I wanted to make a PR to tell users to define their appengine.tools.cloudSdkHome but then I saw there is all this managed SDK code so maybe there's a way to have the setup working without ?

buildscript {
    ext.kotlin_version = '1.2.71'
    repositories {
        jcenter()
        mavenCentral()
    }
    dependencies {
        classpath 'com.google.cloud.tools:appengine-gradle-plugin:2.0.0-rc3'
        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
    }
}

repositories {
    maven {
        url 'https://maven-central.storage.googleapis.com'
    }
    jcenter()
    mavenCentral()
}

apply plugin: 'kotlin'
apply plugin: 'war'
apply plugin: 'com.google.cloud.tools.appengine-standard'

dependencies {
    providedCompile group: 'javax.servlet', name: 'javax.servlet-api', version: '3.1.0'
    compile "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"
    compile "org.jetbrains.kotlin:kotlin-reflect:$kotlin_version"
    compile 'com.google.cloud:google-cloud-datastore:1.14.0'
    compile 'com.squareup.moshi:moshi:1.5.0'
    compile 'com.squareup.moshi:moshi-kotlin:1.5.0'
}

appengine {
    tools {
        cloudSdkHome = "/home/martin/google-cloud-sdk"
    }
    deploy {
        projectId = 'GCLOUD_CONFIG'
        version = 'GCLOUD_CONFIG'
        stopPreviousVersion = true
        promote = true
    }
}
loosebazooka commented 5 years ago

Yeah the managed sdk code is for auto downloading. So if you don't point to a cloud sdk it should download one. I'm not sure exactly why you were getting that validation error (I'm not near a computer), but it could be that some file exists in that location?

martinbonnin commented 5 years ago

Then the README.md is misleading because it states Cloud SDK as a dependancy:

The Cloud SDK is required for this plugin to function. Download and install it before running any tasks.

There's no /home/martin/.cache/google-cloud-tools-java so if I remove the hardcoded cloudSdkHome, nothing gets downloaded.

loosebazooka commented 5 years ago

Ah yes. The docs will be updated when we do a 2.0 final release.

But it's strange that downloading isn't working. I'll go try it when I can. I'll leave this bug open for now

martinbonnin commented 5 years ago

But even if it auto-downloads, there are still manual steps needed to authenticate, right ? Something like gcloud init ? Or are you able to make this interactive ?

loosebazooka commented 5 years ago

There should be options for authenticating. appengineCloudSdkLogin for instance let's you run through the auth flow. And then projectId can be configured in your build.gralde.

martinbonnin commented 5 years ago

I see... Out of curiosity, what's the value of supporting both managed Cloud SDK and manual Cloud SDK installs ? I guess a lot of users will need gcloud in their path anyways ? Having to support both options is some maintenance work as well as some confusion to newcomers like me.

loosebazooka commented 5 years ago

One useful example is in a CI environment, where you might want to limit downloads (for speed or whatever). Yes, you could potentially solve this with a cache, but maybe not all CI's support caching.

martinbonnin commented 5 years ago

But even the managed SDK needs to be downloaded at some point right ? I've been using both travis and bitrise and I'm pretty sure I would need to download the cloud sdk in all cases since I don't have much control over what the base images contain.

loosebazooka commented 5 years ago

Yeah it's an example, but an image could potentially already have the sdk already on it.

JoeWang1127 commented 2 years ago

close as not planned

martinbonnin commented 2 years ago

@JoeWang1127 I guess this is just a wording issue. The current doc says:

The [Cloud SDK](https://cloud.google.com/sdk) is required for this plugin to function.
Versions greater than 2.0.0 will automatically download and install it for you. You may 
also choose to download and install it before running any tasks.

I think the current behaviour is this:

The plugin requires the [gcloud](https://cloud.google.com/sdk) CLI. If `gcloud` 
is found in your PATH, it will be used. Else the plugin will download and install 
`gcloud` automatically. 

Is that an appropriate depiction of the current state? If yes, I can make a PR.

chanseokoh commented 2 years ago

I think the current behavior ignores PATH?

elefeint commented 2 years ago

Reopening because this is indeed confusing. This section describes the behavior well, but "The Cloud SDK is required for this plugin to function. Download and install it before running any tasks." sentence is ambiguous.

We'll accept the documentation contribution -- it can point to the table describing cloudSdkHome/cloudSdkVersion, or say something along the lines of "Cloud CLI will be downloaded automatically, unless cloudSdkHome property is provided.

martinbonnin commented 2 years ago

Pull request there: https://github.com/GoogleCloudPlatform/app-gradle-plugin/pull/434

mpeddada1 commented 2 years ago

Closing this issue given the clarification in the documentation (#434). Please feel free to reopen if there are any further questions.