Added version API #316

milux · 2020-02-19T12:01:03Z

This PR adds support for the version API documented at https://docs.docker.com/engine/api/v1.35/#operation/SystemVersion.
RtVersion doesn't have any methods, since there are no ops available with this API. Convenience methods for the available JSON properties might be implemented if desired.

coveralls · 2020-02-19T12:03:13Z

Pull Request Test Coverage Report for Build 581

16 of 17 (94.12%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.06%) to 90.635%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/main/java/com/amihaiemil/docker/RtVersion.java	13	14	92.86%

Totals
Change from base Build 575:	0.06%
Covered Lines:	871
Relevant Lines:	961

💛 - Coveralls

amihaiemil

@milux Please see my comments.

Also, did you make these PRs because of this article?
https://amihaiemil.com/2020/02/15/solve-github-issues-and-get-cash.html

If yes, please be aware that there are no open bounties for this repo.
However, if the PRs get merged, I'll send you some cash...

But please, next time, don't work unless you are assigned and the ticket has a bounty on it.

amihaiemil · 2020-02-19T12:14:01Z

src/main/java/com/amihaiemil/docker/RtDocker.java

+    public Version version() throws IOException {
+        final String versionUri = this.baseUri.toString() + "/version";
+        final HttpGet version = new HttpGet(versionUri);
+        final JsonObject json = this.client.execute(


@milux This variable is only used one time, please put it inline, in the call to new RtVersion(), like this:

return new RtVersion( this.client.execute( version, new ReadJsonObject( new MatchStatus(version.getURI(), HttpStatus.SC_OK) ) ), this.client, URI.create(versionUri), this );

Also, wrap everything in try/finally and release the connection, like here

Done, also improved structure according to the pattern I found in Inspection.java.

amihaiemil · 2020-02-19T12:15:04Z

src/main/java/com/amihaiemil/docker/Version.java

+ * @since 0.0.11
+ */
+public interface Version extends JsonObject {
+}


@milux We should also have accesor methods, since the JsonObject has quite a few attributes, as I see.
Also, it would be nice to have the docker() method as well, which returns the parent Docker instance.

If you already spent too much time, please leave a puzzle for continuing implementation.

Done, plus I have no clue what those "puzzles" are? Read about it somewhere in your code, but didn't quite get it. Can you provide an example for better understanding?

Removed the local variables because I think they're pointless anyway. Can undo this if you think differently.

@milux Can you remove the get and is prefixes from the methods name? I never use them, they are just noise in the code :)

I also don't see the docker() method, can you add it?

amihaiemil · 2020-02-19T12:15:31Z

src/main/java/com/amihaiemil/docker/RtVersion.java

+        this.baseUri = uri;
+        this.docker = dkr;
+    }
+}


@milux We also need unit tests. See how other unit tests are written, it's not complicated. Generally, PRs should not decrease code coverage.

Done, although the method I wrote the unit test is somewhat different from that of the other tests. (Using locally stored JSON resource file for response mockup, for instance.) Hope you're still fine with this...

@milux Yes, it's fine. It can be changed later, no prob.

0crat · 2020-02-19T12:18:31Z

This pull request #316 is assigned to @amihaiemil /z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @amihaiemil /z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be a monetary reward for this job

0crat · 2020-02-19T12:29:38Z

Manual assignment of issues is discouraged, see §19: -5 point(s) just awarded to @amihaiemil /z

milux · 2020-02-19T14:38:33Z

BTW: Please let me be quite clear that I don't want to earn money with this.
Also didn't know about your article, but thanks for the information. 😄
This is an improvement for my institute because we need this API in a project, and I chose your library because its lightweight dependency tree. 😉

amihaiemil · 2020-02-19T14:45:06Z

@milux Thank you. I would still like to send you some cash in PayPal, if you give me your e-mail address from there.

I want to invest in my projects, attract developers, build a distributed team etc.

Also, maybe your institute could invest a few dollars in this library, so we make it better with Bounties.

amihaiemil

@milux Thank you! Just remove the is and get prefixes and add the docker() method.

A "puzzle" is actually a TODO which will automatically be transformed into a new Github Issue. It looks like this:

@todo #parent_ticket_number:30min Please continue with implementation,
 we need this and that, and such a Puzzle should have at least 2-3 sentences,
 otherwise the bot will complain. Also pay attention to the puzzle's indentation,
 the second and following sentences have to begin with a space (be alligned with
 the @ sign).

amihaiemil · 2020-02-19T14:47:19Z

src/main/java/com/amihaiemil/docker/RtVersion.java

+     * @throws IOException If an I/O error occurs.
+     */
+    RtVersion(final HttpClient client, final URI uri) throws IOException {
+        super(fetch(client, uri));


@milux Well, this fetch method goes a little against the architecture. How you did it before, it was ok. But I can change it later without modifying the interfaces, so it's ok.

amihaiemil · 2020-02-19T14:47:41Z

src/main/java/com/amihaiemil/docker/RtVersion.java

+        this.baseUri = uri;
+        this.docker = dkr;
+    }
+}


@milux Yes, it's fine. It can be changed later, no prob.

amihaiemil · 2020-02-19T14:48:42Z

src/main/java/com/amihaiemil/docker/Version.java

+ * @since 0.0.11
+ */
+public interface Version extends JsonObject {
+}


@milux Can you remove the get and is prefixes from the methods name? I never use them, they are just noise in the code :)

I also don't see the docker() method, can you add it?

milux · 2020-02-19T15:03:54Z

Done, both. Regarding docker(): Not possible anymore, because I removed the property. 😉
Also can't think about a use case where this is actually useful, but if you insist, I will add it back...

amihaiemil · 2020-02-19T15:07:03Z

@milux well, all the objects in the library have the docker() method. It's very useful. Say you pass this Version object to a method in your software, and inside that method you want to access Docker. You just say version.docker() -- this method call would translate to "Get me the Docker with this version".

But we can add it later, no worries.

Can you fix the build, then we'll merge the PR? :D

milux · 2020-02-19T15:10:54Z

@milux Thank you. I would still like to send you some cash in PayPal, if you give me your e-mail address from there.

I want to invest in my projects, attract developers, build a distributed team etc.

Sure, very good and noble attitude and plans, but I'm afraid taking money could violate a bunch of regulations, for instance our anti-corruption rules (Yes it's silly, but that's Germany... ^^).

Also, maybe your institute could invest a few dollars in this library, so we make it better with Bounties.

I wish, but I believe the sad reality is that while we can pay for commercial tools, supporting FOSS projects bounces because all those stupid regulations. 😞

amihaiemil · 2020-02-19T15:15:51Z

@milux Well, I don't think it's corruption to take money for honest work :D

If you change your mind, let me know. I saw you understood the architecture here so we could use contributors.

amihaiemil · 2020-02-19T15:16:07Z

@rultor try to merge

amihaiemil · 2020-02-19T15:16:24Z

@rultor try to merge

rultor · 2020-02-19T15:50:05Z

@rultor try to merge

@amihaiemil OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2020-02-19T15:56:25Z

@rultor try to merge

@amihaiemil @milux Oops, I failed. You can see the full log here (spent 6min)

WARNING: Your kernel does not support swap limit capabilities or the cgroup is not mounted. Memory limited without swap.
+ set -e
+ set -o pipefail
+ shopt -s dotglob
+ useradd -m -G sudo r
+ echo '%sudo ALL=(ALL) NOPASSWD:ALL'
+ cp -R /root/.bashrc /root/.composer /root/.gem /root/.gnupg /root/.m2 /root/.oracle_jre_usage /root/.profile /root/texmf /home/r
+ cp -R ./cid ./end.sh ./entry.sh ./pid ./pubring.gpg ./repo ./run.sh ./script.sh ./secring.gpg ./settings.xml ./stdout /home/r
+ rm -rf repo
+ chown -R r:r /home/r
+ chmod a+x /home/r/script.sh
+ su --login r --command /home/r/script.sh
mesg: /dev/pts/0: Operation not permitted
+ set -e
+ set -o pipefail
+ shopt -s expand_aliases
+ alias 'sudo=sudo -i'
+ export HOME=/home/r
+ HOME=/home/r
+ cd /home/r/repo
+ export 'MAVEN_OPTS=-XX:MaxPermSize=256m -Xmx1g'
+ MAVEN_OPTS='-XX:MaxPermSize=256m -Xmx1g'
+ export [email protected]:amihaiemil/docker-java-api.git
+ [email protected]:amihaiemil/docker-java-api.git
+ export pull_id=316
+ pull_id=316
+ export [email protected]:milux/docker-java-api.git
+ [email protected]:milux/docker-java-api.git
+ export fork_branch=version-api
+ fork_branch=version-api
+ export head_branch=master
+ head_branch=master
+ export 'pull_title=Added version API'
+ pull_title='Added version API'
+ export author=amihaiemil
+ author=amihaiemil
+ mvn clean install -Pcheckstyle
Java HotSpot(TM) 64-Bit Server VM warning: ignoring option MaxPermSize=256m; support was removed in 8.0
[INFO] Scanning for projects...
[INFO]                                                                         
[INFO] ------------------------------------------------------------------------
[INFO] Building Docker Java API 0.0.11-SNAPSHOT
[INFO] ------------------------------------------------------------------------
Downloading: http://repo.maven.apache.org/maven2/org/jacoco/jacoco-maven-plugin/0.7.6.201602180812/jacoco-maven-plugin-0.7.6.201602180812.pom

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 0.695 s
[INFO] Finished at: 2020-02-19T15:54:49+00:00
[INFO] Final Memory: 9M/457M
[INFO] ------------------------------------------------------------------------
[ERROR] Plugin org.jacoco:jacoco-maven-plugin:0.7.6.201602180812 or one of its dependencies could not be resolved: Failed to read artifact descriptor for org.jacoco:jacoco-maven-plugin:jar:0.7.6.201602180812: Could not transfer artifact org.jacoco:jacoco-maven-plugin:pom:0.7.6.201602180812 from/to Maven (http://repo.maven.apache.org/maven2): Failed to transfer file: http://repo.maven.apache.org/maven2/org/jacoco/jacoco-maven-plugin/0.7.6.201602180812/jacoco-maven-plugin-0.7.6.201602180812.pom. Return code is: 501 , ReasonPhrase:HTTPS Required. -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginResolutionException
container 482ec32d8e0e24d25f253af32ed7399e5a3aee4ef07e0df304b4afaed821050d is dead
Wed Feb 19 16:56:11 CET 2020

rultor · 2020-02-19T16:00:14Z

@rultor try to merge

@amihaiemil OK, I'll try to merge now. You can check the progress of the merge here

milux · 2020-02-19T16:04:28Z

@amihaiemil I noticed that there was a copy-paste flaw in the comment of the unit test I provided. It appears that the commit didn't make it to this PR. If rultor fails again, you may want to include this, although it's a minor thing.

rultor · 2020-02-19T16:06:42Z

@rultor try to merge

@amihaiemil @milux Oops, I failed. You can see the full log here (spent 6min)

WARNING: Your kernel does not support swap limit capabilities or the cgroup is not mounted. Memory limited without swap.
+ set -e
+ set -o pipefail
+ shopt -s dotglob
+ useradd -m -G sudo r
+ echo '%sudo ALL=(ALL) NOPASSWD:ALL'
+ cp -R /root/.bashrc /root/.composer /root/.gem /root/.gnupg /root/.m2 /root/.oracle_jre_usage /root/.profile /root/texmf /home/r
+ cp -R ./cid ./end.sh ./entry.sh ./pid ./pubring.gpg ./repo ./run.sh ./script.sh ./secring.gpg ./settings.xml ./stdout /home/r
+ rm -rf repo
+ chown -R r:r /home/r
+ chmod a+x /home/r/script.sh
+ su --login r --command /home/r/script.sh
mesg: /dev/pts/0: Operation not permitted
+ set -e
+ set -o pipefail
+ shopt -s expand_aliases
+ alias 'sudo=sudo -i'
+ export HOME=/home/r
+ HOME=/home/r
+ cd /home/r/repo
+ export 'MAVEN_OPTS=-XX:MaxPermSize=256m -Xmx1g'
+ MAVEN_OPTS='-XX:MaxPermSize=256m -Xmx1g'
+ export [email protected]:amihaiemil/docker-java-api.git
+ [email protected]:amihaiemil/docker-java-api.git
+ export pull_id=316
+ pull_id=316
+ export [email protected]:milux/docker-java-api.git
+ [email protected]:milux/docker-java-api.git
+ export fork_branch=version-api
+ fork_branch=version-api
+ export head_branch=master
+ head_branch=master
+ export 'pull_title=Added version API'
+ pull_title='Added version API'
+ export author=amihaiemil
+ author=amihaiemil
+ mvn clean install -Pcheckstyle
Java HotSpot(TM) 64-Bit Server VM warning: ignoring option MaxPermSize=256m; support was removed in 8.0
[INFO] Scanning for projects...
[INFO]                                                                         
[INFO] ------------------------------------------------------------------------
[INFO] Building Docker Java API 0.0.11-SNAPSHOT
[INFO] ------------------------------------------------------------------------
Downloading: http://repo.maven.apache.org/maven2/org/jacoco/jacoco-maven-plugin/0.7.6.201602180812/jacoco-maven-plugin-0.7.6.201602180812.pom

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 0.556 s
[INFO] Finished at: 2020-02-19T16:04:57+00:00
[INFO] Final Memory: 8M/362M
[INFO] ------------------------------------------------------------------------
[ERROR] Plugin org.jacoco:jacoco-maven-plugin:0.7.6.201602180812 or one of its dependencies could not be resolved: Failed to read artifact descriptor for org.jacoco:jacoco-maven-plugin:jar:0.7.6.201602180812: Could not transfer artifact org.jacoco:jacoco-maven-plugin:pom:0.7.6.201602180812 from/to Maven (http://repo.maven.apache.org/maven2): Failed to transfer file: http://repo.maven.apache.org/maven2/org/jacoco/jacoco-maven-plugin/0.7.6.201602180812/jacoco-maven-plugin-0.7.6.201602180812.pom. Return code is: 501 , ReasonPhrase:HTTPS Required. -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginResolutionException
container d17f0155deb364a2edaa1ec8eb29087f64edc84af22f4e7d497176e1f3048345 is dead
Wed Feb 19 17:06:29 CET 2020

amihaiemil · 2020-02-19T16:08:44Z

@milux rultor has other issues, unfortunately.... it will be an Issue for us now because until Rultor is fixed, I cannot make a release to Maven Central...

0crat · 2020-02-19T16:51:13Z

Job was finished in 3 hours, bonus for fast delivery is possible (see §36)

0crat · 2020-02-19T17:51:10Z

Order was finished: +20 point(s) just awarded to @amihaiemil /z

0crat · 2020-02-19T18:16:54Z

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @amihaiemil /z

Added version API

83bad3e

0crat added scope role/REV labels Feb 19, 2020

amihaiemil self-assigned this Feb 19, 2020

amihaiemil requested changes Feb 19, 2020

View reviewed changes

Improved version API implementation and provided unit test

d04a543

amihaiemil requested changes Feb 19, 2020

View reviewed changes

milux added 2 commits February 19, 2020 15:58

Added missing override annotations

9dc7a66

Removed is/get method prefixes

3c196fd

Method name fixed according to checkstyle rules

f58d1b0

amihaiemil merged commit 5f42e0c into amihaiemil:master Feb 19, 2020

0crat removed the scope label Feb 19, 2020

amihaiemil mentioned this pull request Feb 21, 2020

Release 0.0.11 #317

Closed

Uh oh!

Added version API #316

Added version API #316

Uh oh!

Conversation

milux commented Feb 19, 2020

Uh oh!

coveralls commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 581

💛 - Coveralls

Uh oh!

amihaiemil left a comment

Choose a reason for hiding this comment

Uh oh!

amihaiemil Feb 19, 2020 • 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!

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!

0crat commented Feb 19, 2020

Uh oh!

0crat commented Feb 19, 2020

Uh oh!

milux commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amihaiemil commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amihaiemil left a comment

Choose a reason for hiding this comment

Uh oh!

amihaiemil Feb 19, 2020 • 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!

milux commented Feb 19, 2020

Uh oh!

amihaiemil commented Feb 19, 2020

Uh oh!

milux commented Feb 19, 2020

Uh oh!

amihaiemil commented Feb 19, 2020

Uh oh!

amihaiemil commented Feb 19, 2020

Uh oh!

amihaiemil commented Feb 19, 2020

Uh oh!

rultor commented Feb 19, 2020

Uh oh!

rultor commented Feb 19, 2020

Uh oh!

rultor commented Feb 19, 2020

Uh oh!

milux commented Feb 19, 2020

Uh oh!

rultor commented Feb 19, 2020

Uh oh!

amihaiemil commented Feb 19, 2020

Uh oh!

coveralls commented Feb 19, 2020 •

edited

Loading

amihaiemil Feb 19, 2020 •

edited

Loading

milux commented Feb 19, 2020 •

edited

Loading

amihaiemil commented Feb 19, 2020 •

edited

Loading

amihaiemil Feb 19, 2020 •

edited

Loading