Ore Deploy plugin by windy1 · Pull Request #9 · SpongePowered/SpongeGradle

windy1 · 2017-03-28T22:40:43Z

Corresponding Ore branch: https://github.com/SpongePowered/Ore/tree/feature/gradle-deploy

Signed-off-by: Walker Crouse walkercrouse@hotmail.com

simon816 · 2017-03-28T23:21:04Z

+
+class OreDeployExtension {
+
+    String instanceUrl = 'http://localhost:9000'


Probably makes more sense to default this to https://ore.spongepowered.org

simon816 · 2017-03-28T23:32:06Z

Am I right in thinking that we were going to have a maven deploy endpoint on Ore? (i.e. running an uploadArchives task)

Is this gradle integration a replacement for that or just a 'filler' before full maven support is added?
Edit: or in addition to maven support?

windy1 · 2017-03-29T00:17:24Z

@simon816 I think maven support could potentially be built on top of this, but I wanted something that just hooked into the current system as well for simplicity's sake. Working maven into the current file system is a much larger undertaking.

stephan-gh · 2017-03-29T12:53:24Z

+
+            conn.connect()
+            if (conn.responseCode != HttpURLConnection.HTTP_CREATED) {
+                println '[failure] ' + conn.responseCode + ' ' + conn.responseMessage


You should use the logger of the task here (just get the logger property of the task)

stephan-gh · 2017-03-29T12:57:06Z

+        project.with {
+            extensions.create('oreDeploy', OreDeployExtension)
+            def task = task('oreDeploy', type: OreDeployTask)
+            task.dependsOn('signArchives')


What if this task isn't defined? Should we automatically apply and configure the signing plugin?

Probably not considering, their pub/priv keys will need to be configured properly by the user and will likely just cause confusion IMO.

stephan-gh · 2017-03-29T12:58:58Z

+
+            os = conn.outputStream
+            requestEntity.writeTo(os)
+            os.flush()


Groovy has a convenient OutputStream.withStream method that will automatically handle closing the stream

stephan-gh · 2017-03-31T13:08:22Z

+                printErrorStream is
+                is.close()
+                is = null
+                throw new GradleException()


You should also use conn.errorStream.withStream here. Also, instead of printing the error to the logger and then throwing the exception you should include the error message in the exception. Otherwise, Gradle will display an error with (no error message)

I actually tried using withStream with the InputStreams but it was behaving very strangely. AFAI could tell it was down-casting the stream to the HttpURLConnection anonymous implementing class and then couldn't find a method declaration for printErrorStream with that sub-class. Very odd.

stephan-gh · 2017-03-31T13:09:17Z

+        } catch (InterruptedIOException | SocketException ignored) {
+            throw new GradleException('Failed to connect to Ore.')
+        } catch (SSLHandshakeException ignored) {
+            logger.error('[failure] Outdated Java!')


I think it would make more sense if we include this message in the exception below

stephan-gh · 2017-03-31T13:14:18Z

+        if (channel == null) {
+            throw new InvalidUserDataException("Missing channel name.")
+        }
+        HttpEntity requestEntity = MultipartEntityBuilder.create()


You use the API from Apache's http client here, but send the request using Java's API. Since the full http client is downloaded for this anyway, would it be nicer to use Apache's http client entirely for sending the request?

Sure, I really just used this API because writing multipart form data is a huge PITA from scratch (I've done it before). I quick Google search says that HttpClient is optimized for performance however so I have nothing against switching.

Signed-off-by: Walker Crouse <walkercrouse@hotmail.com>

stephan-gh

Thanks for switching the http client 👍

stephan-gh · 2017-04-01T07:36:39Z

+            throw new IOException('An unexpected error occurred.', e)
+        } finally {
+            if (response != null) {
+                response.close()


Groovy has .withCloseable similar to .withStream that does this automatically

stephan-gh · 2017-04-01T07:36:44Z

+            if (response != null) {
+                response.close()
+            }
+            http.close()


I'm having the same issue with the response entity's InputStream being down-casted when in the closure again for some reason.

stephan-gh · 2017-04-01T07:39:04Z

+        logger.debug(requestEntity.toString())
+
+        HttpClient http = HttpClients.createDefault()
+        HttpPost post = new HttpPost(projectUrl.toURI())


The constructor accepts a String so you can just pass in the string url directly here instead of parsing it into an URL first.

Yeah but I would rather catch the exception for a malformed URL to provide a more useful error.

windy1 · 2017-04-10T18:25:24Z

@Minecrell Can you think of anything else before this goes in?

stephan-gh · 2017-04-10T18:27:19Z

@windy1 Not right now, but if you don't mind waiting until tomorrow I'd still like to look at some of the Gradle implementation details.

windy1 · 2017-04-10T18:32:56Z

Sure, no problem. 👍

kashike · 2017-04-10T18:47:58Z

+    void run() {
+        Logger logger = project.logger
+        def artifacts = deploy.allArtifacts
+        PublishArtifact plugin = artifacts.find { a -> (a.type == "jar") }


Which artifact is uploaded? Is it configurable?

It looks like it publishes everything in the archives block if it's a jar.

A lot of plugins have sources, javadocs, raw, and then dist jars in there. It'd be a massive improvement to allow specifying what is published to Ore separately.

Asherslab · 2017-10-17T00:32:34Z

This looks like an absolutely awesome thing, is this ready to go? I'd love to see this soon

windy1 · 2017-10-25T01:37:22Z

@Asherslab It was working last time I tested it, could probably be improved upon, but could probably go in.

Asherslab · 2017-10-25T06:25:31Z

Sounds awesome! I could really use this whenever it gets merged in. Thanks for the work!

Asherslab · 2017-10-31T08:29:37Z

So, i take it this just needs a review from someone else to get merged?
(If there's anyway i can use it now that'd be cool too, but either way) 😄

windy1 · 2017-10-31T18:51:31Z

@Asherslab Essentially, you can build the project yourself if you're like to use it. Should work.

Asherslab · 2017-11-01T05:40:33Z

Hi @windy1, thanks, i successfully built it myself just after i sent that (sorry)

~~i do have a bit of an error though, and i'm not sure if that's my fault or a bug:~~

Gradle Error: https://gist.github.com/Asherslab/a7b357a2e58becace55eca2dc292f4e6
build.gradle (With sensitive info removed): https://github.com/Minecolonies/Rankup/blob/API-6/feature/concurrant-tracks/build.gradle
(I also tried adding the line oreDeploy.instanceUrl = "https://ore.spongepowered.org/Asherslab/Rankup" incase that was part of the error)

EDIT:
I just realised that I'm an idiot and didn't add the apache client dependency to my build.gradle! learning learning, sorry for the trouble

i would post this as an issue in your fork, but you don't have an issues tab there. and i feel like making an issue for a non-merged PR here wouldn't be okay (Feel free to tell me to make an issue here 😄 )

Thanks for your work!

simon816 reviewed Mar 28, 2017

View reviewed changes

stephan-gh reviewed Mar 29, 2017

View reviewed changes

stephan-gh self-requested a review March 31, 2017 12:59

stephan-gh reviewed Mar 31, 2017

View reviewed changes

Ore Deploy plugin

02451ce

Signed-off-by: Walker Crouse <walkercrouse@hotmail.com>

stephan-gh reviewed Apr 1, 2017

View reviewed changes

windy1 changed the title ~~[WIP] Ore Deploy plugin~~ Ore Deploy plugin Apr 10, 2017

kashike reviewed Apr 10, 2017

View reviewed changes

progwml6 mentioned this pull request Feb 25, 2018

Ore V2 API design SpongePowered/Ore#449

Open

Zidane merged commit 0030e17 into SpongePowered:master Feb 26, 2018


		class OreDeployExtension {

		String instanceUrl = 'http://localhost:9000'

Conversation

windy1 commented Mar 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simon816 commented Mar 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

windy1 commented Mar 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

windy1 Mar 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

windy1 Mar 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephan-gh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

windy1 commented Apr 10, 2017

Uh oh!

stephan-gh commented Apr 10, 2017

Uh oh!

windy1 commented Apr 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Asherslab commented Oct 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

windy1 commented Oct 25, 2017

Uh oh!

Asherslab commented Oct 25, 2017

Uh oh!

Asherslab commented Oct 31, 2017

Uh oh!

windy1 commented Oct 31, 2017

Uh oh!

Asherslab commented Nov 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

simon816 commented Mar 28, 2017 •

edited

Loading

windy1 Mar 29, 2017 •

edited

Loading

windy1 Mar 31, 2017 •

edited

Loading

Asherslab commented Oct 17, 2017 •

edited

Loading

Asherslab commented Nov 1, 2017 •

edited

Loading