-
Notifications
You must be signed in to change notification settings - Fork 5
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
UCR Support #15
UCR Support #15
Conversation
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.
left some comments. will have a debug tomorrow. very good overall <3
// } | ||
// CloseableHttpResponse response = client.execute(put); | ||
// getLog().info("Response from DC/OS [" + response.getStatusLine().getStatusCode() + "] " + IOUtils.toString(response.getEntity().getContent(), "UTF-8")); | ||
|
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.
Why removing this code block? Am I missing something?
String nexusPassword; | ||
|
||
|
||
@Parameter(defaultValue = "${project.basedir}/.dcos-token", property = "dcosTokenFile", required = true) |
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.
Please indent line 31 to 40 to two spaces.
|
||
getLog().info(project.getArtifact().getFile().toString()); | ||
|
||
Process p = Runtime.getRuntime().exec("curl -u " + nexusUser + ":" + nexusPassword +" --upload-file "+ project.getArtifact().getFile().toString() + " http://" + nexusUrl + "/repository/" + nexusRepositoryName + "/");//file.toString()); |
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.
Ok, I`ll have a look at this tomorrow. Nice hack btw ;P
In this process world, I would call p.exitCode
to make sure this process finished.
throw new RuntimeException("Artifact does not exist. Did you run mvn package dcos:pushArtifact?"); | ||
} | ||
|
||
tmpPath = File.createTempFile("ucr-deploy", "json").toPath(); |
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.
@joerg84 I changed this to work in os tmp directory, hope this is ok for you.
|
||
getLog().info("Uploading this file: " + file.toString()); | ||
|
||
HttpPut put = new HttpPut("http://" + nexusUrl + "/repository/" + nexusRepositoryName + "/" + file.getName()); |
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.
file.getName()
was missing here. curl --upload-file
adds the filename to the url.
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.
Shouldn't the <nexusUrl>
parameter also be including the protocol? Hardcoding it to http
seems a bit weak?
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.
yep, sounds good. Maybe it would be possible to do things like:
if nexusUrl includes protocol, use it, use http as fallback otherwise? Are you fancy providing a small patch or is @joerg84 willing to update the implementation? ❤️
Thank you, you rock! 🤘 |
No description provided.