Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add two basic templates (google-native + raw kotlin) #351

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add two basic templates (google-native + raw kotlin) #351

wants to merge 1 commit into from

Conversation

myhau
Copy link
Member

@myhau myhau commented Sep 22, 2023 •
edited

Task

Resolves: #350

Description

Example usage

Now:

pulumi new https://github.com/VirtuslabRnD/pulumi-kotlin/tree/350-add-templates/templates/google-native

Once it's merged:

pulumi new https://github.com/VirtuslabRnD/pulumi-kotlin/tree/main/templates/google-native

Potentially, after moving to VirtusLab GH org:

pulumi new https://github.com/VirtusLab/pulumi-kotlin/tree/main/templates/google-native

With "Deploy with Pulumi" button:

Deploy with Pulumi

Screenshot 2023-09-22 at 19 51 35

Source:

[![Deploy with Pulumi](https://get.pulumi.com/new/button.svg)](https://app.pulumi.com/new?template=[{template-url}](https://github.com/VirtuslabRnD/pulumi-kotlin/tree/350-add-templates/templates/google-native))

To do

  • Decide if we should communicate this now or after we move to VirtusLab GH organization
  • Update docs
  • (maybe?) Add Deploy with Pulumi buttons (docs)

Suggestions needed

  • Should we suffix all these with -kotlin? For example, which name is better google-native-kotlin or google-native
  • How should we name the basic (almost empty) template. kotlin vs basic vs default vs empty

@myhau myhau marked this pull request as ready for review September 22, 2023 17:52
@myhau myhau requested a review from a team as a code owner September 22, 2023 17:52
@myhau myhau self-assigned this Sep 22, 2023
@jplewa
Copy link
Member

jplewa commented Sep 25, 2023

(maybe?) Add Deploy with Pulumi buttons (docs)

I like them!

Should we suffix all these with -kotlin? For example, which name is better google-native-kotlin or google-native
How should we name the basic (almost empty) template. kotlin vs basic vs default vs empty

Looking at https://github.com/pulumi/templates/, most of them have a suffix with the language:

image

On one hand, this doesn't really concern us, since our templates are hosted in this repository (and this repository has kotlin in its name), buuut (as seen in your screenshot) the actual template name right now is google-native with no mention of Kotlin:
image
I think I would add -kotlin to the name? And then the basic template can stay called kotlin.

Other stuff:

  1. One thing I'm not crazy about in the google-native template is that we're using the same code snippet as in out examples/E2E tests, so this code is duplicated (if we did the same with GCP it would be even worse, since we also have the same snippet in the README). Maybe something more minimal? In the gcp-java template they just do a bucket (https://github.com/pulumi/templates/blob/master/gcp-java/src/main/java/myproject/App.java), maybe we could also do that?
  2. Another concern of mine is that while with the examples it's not a huge issue if we don't use the most up-to-date version of each library, I think it would make sense to do that in the templates? Everything else updates automatically, but these we would have to update manually and I don't love that. I'm not saying that's a deal breaker, because I don't know how to automate it nicely.

jplewa
jplewa reviewed Sep 25, 2023
View reviewed changes
templates/google-native/app/build.gradle.kts

dependencies {
implementation("org.virtuslab:pulumi-kotlin:0.9.4.0")
implementation("org.virtuslab:pulumi-google-native-kotlin:0.31.1.1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? In the E2E tests I encountered an issue where I has to exclude some stuff due to a versioning conflict

pulumi-kotlin/examples/google-native-sample-project/pom.xml

Lines 21 to 43 in f01ac54

<dependencies>
<dependency>
<groupId>org.virtuslab</groupId>
<artifactId>pulumi-google-native-kotlin</artifactId>
<version>0.31.0.0-SNAPSHOT</version>
<exclusions>
<exclusion>
<groupId>com.pulumi</groupId>
<artifactId>pulumi</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.pulumi</groupId>
<artifactId>pulumi</artifactId>
<version>0.9.4</version>
</dependency>
<dependency>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-stdlib</artifactId>
<version>${kotlin.version}</version>
</dependency>
</dependencies>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, aside from the potential dependency conflict, it shouldn't be necessary to include the core SDK explicitly, just the Google Native dependency should be fine.

@jplewa
Copy link
Member

jplewa commented Sep 25, 2023

Btw, I just realized another thing. Our examples directory uses SNAPSHOT versions of our libraries (and it's not even the most recent ones, since we rely on those gist-hosted subschemas). Each example has a README where we explain to the user how to run this code, but we don't take into account that the code won't actually run with this dependency? I think we need to create a task to address that. Maybe we need to place regular up-to-date versions in our examples and replace the version (or the entire pom.xml) before running E2E tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jplewa jplewa jplewa left review comments

@ddzikon ddzikon Awaiting requested review from ddzikon ddzikon is a code owner automatically assigned from VirtuslabRnD/jvm-lab

At least 1 approving review is required to merge this pull request.

Assignees

@myhau myhau

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add easy to reference pulumi-kotlin template in this repository
2 participants
@myhau @jplewa