You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "Cole-Greer (via GitHub)" <gi...@apache.org> on 2023/03/29 21:42:27 UTC
[GitHub] [tinkerpop] Cole-Greer opened a new pull request, #2007: TINKERPOP-2852 Deploying multi-arch docker images
Cole-Greer opened a new pull request, #2007:
URL: https://github.com/apache/tinkerpop/pull/2007
Replaces spotify plugin with io.fabric8 docker-maven-plugin. The new plugin has the benefit of supporting multi-arch images through buildx for deployments and it is an actively maintained plugin as opposed to the spotify plugin.
The new plugin continues to fetch auth credentials from the same location so no changes to the release environment will be necessary.
I have tested the full set of .Net gherkin and integration tests against the new image with no issues. I have also used [container-diff](https://github.com/GoogleContainerTools/container-diff) to compare the new image to the one that we currently build using the mvn exec plugin and docker compose. These are the results:
```
-----File-----
These entries have been added to colegreer/tinkerpop:server-3.5.6-SNAPSHOT: None
These entries have been deleted from colegreer/tinkerpop:server-3.5.6-SNAPSHOT: None
These entries have been changed between colegreer/tinkerpop:server-3.5.6-SNAPSHOT and tinkerpop/gremlin-server:3.5.6-SNAPSHOT:
FILE SIZE1 SIZE2
/opt/gremlin-server/lib/gremlin-server-3.5.6-SNAPSHOT.jar 243K 242.8K
/etc/ssl/certs/java/cacerts 155.3K 155.3K
-----Apt-----
Packages found only in colegreer/tinkerpop:server-3.5.6-SNAPSHOT: None
Packages found only in tinkerpop/gremlin-server:3.5.6-SNAPSHOT: None
Version differences: None
```
I haven't yet identified the minute difference in the jar file between the two images. I can investigate this further if anyone views it as a potential concern.
TODO:
Switch gremlin-console to new plugin.
Support manual image deployment as similar to what `mvn dockerfile:push` currently does.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] lyndonbauto commented on a diff in pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "lyndonbauto (via GitHub)" <gi...@apache.org>.
lyndonbauto commented on code in PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#discussion_r1174226024
##########
gremlin-server/pom.xml:
##########
@@ -32,6 +32,8 @@ limitations under the License.
-->
<maven.exec.skip>false</maven.exec.skip> <!-- default -->
<skipImageBuild>${maven.exec.skip}</skipImageBuild>
+ <docker.platforms>linux/amd64,linux/arm64/v8</docker.platforms>
+
Review Comment:
nit:
```suggestion
<docker.platforms>linux/amd64,linux/arm64/v8</docker.platforms>
```
##########
gremlin-server/pom.xml:
##########
@@ -207,6 +209,39 @@ limitations under the License.
</executions>
</plugin>
</plugins>
+
+ <pluginManagement>
Review Comment:
```suggestion
<pluginManagement>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] xiazcy commented on pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#issuecomment-1522633044
VOTE +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] codecov-commenter commented on pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#issuecomment-1499528300
## [Codecov](https://codecov.io/gh/apache/tinkerpop/pull/2007?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#2007](https://codecov.io/gh/apache/tinkerpop/pull/2007?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b937b3e) into [3.5-dev](https://codecov.io/gh/apache/tinkerpop/commit/8267422b1e583f26797425b22ace0789f3be1e6d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8267422) will **increase** coverage by `0.00%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## 3.5-dev #2007 +/- ##
==========================================
Coverage 69.37% 69.37%
- Complexity 8959 8966 +7
==========================================
Files 866 866
Lines 41183 41219 +36
Branches 5423 5432 +9
==========================================
+ Hits 28570 28595 +25
- Misses 10704 10712 +8
- Partials 1909 1912 +3
```
[see 13 files with indirect coverage changes](https://codecov.io/gh/apache/tinkerpop/pull/2007/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on a diff in pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "FlorianHockmann (via GitHub)" <gi...@apache.org>.
FlorianHockmann commented on code in PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#discussion_r1162603370
##########
pom.xml:
##########
@@ -345,6 +345,55 @@ limitations under the License.
<failIfNoMatch>false</failIfNoMatch>
</configuration>
</execution>
+ <execution>
+ <!-- sets the true.when.is.prerelease.version property to true if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-is-prerelease-version2</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>true.when.is.prerelease.version</name>
+ <value>${only.when.is.prerelease.version}</value>
+ <regex>${project.version}</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.majorVersion.minorVersion property to
+ ${parsedVersion.majorVersion}.${parsedVersion.minorVersion} if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-set-docker-version-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.majorVersion.minorVersion</name>
+ <value>${parsedVersion.majorVersion}.${parsedVersion.minorVersion}${true.when.is.prerelease.version}</value>
+ <regex>.*true</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.latest property to "latest" if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-set-docker-latest-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.latest</name>
Review Comment:
I guess we have to remove this part for `3.5-dev` and only add it for `3.6-dev` as `latest` should always point to the latest `3.6` image, but not a `3.5` one.
##########
pom.xml:
##########
@@ -345,6 +345,55 @@ limitations under the License.
<failIfNoMatch>false</failIfNoMatch>
</configuration>
</execution>
+ <execution>
+ <!-- sets the true.when.is.prerelease.version property to true if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-is-prerelease-version2</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>true.when.is.prerelease.version</name>
+ <value>${only.when.is.prerelease.version}</value>
+ <regex>${project.version}</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.majorVersion.minorVersion property to
Review Comment:
Isn't this exactly the other way around? The property should only bet set to `major.minor` if it is **not** a prerelease version. (same below for `latest`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on a diff in pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "FlorianHockmann (via GitHub)" <gi...@apache.org>.
FlorianHockmann commented on code in PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#discussion_r1176113134
##########
docs/src/upgrade/release-3.5.x.asciidoc:
##########
@@ -30,24 +30,8 @@ complete list of all the modifications that are part of this release.
=== Upgrading for Users
-=== Upgrading for Providers
-
-==== Local steps should handle non-Iterables
-
-`index` steps and local steps `count`, `dedup`, `max`, `mean`, `min`, `order`, `range`, `sample`, `sum`, `tail` should
-work correctly with not only `Iterable` input, but also with arrays and single values.
-
-Examples of queries that should be supported:
-
-[source,groovy]
-----
-g.inject(1).max(local)
-----
-
-[source,java]
-----
-g.inject(new Integer[] {1,2},3).max(Scope.local).toList()
-----
+Gremlin server docker image now supports both AMD64 and ARM64. Multi-architecture image can now be found at
Review Comment:
Could you please add a heading for this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] lyndonbauto commented on a diff in pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "lyndonbauto (via GitHub)" <gi...@apache.org>.
lyndonbauto commented on code in PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#discussion_r1174226183
##########
gremlin-server/pom.xml:
##########
@@ -378,60 +412,28 @@ limitations under the License.
</configuration>
</plugin>
<plugin>
- <groupId>com.spotify</groupId>
- <artifactId>dockerfile-maven-plugin</artifactId>
+ <groupId>io.fabric8</groupId>
+ <artifactId>docker-maven-plugin</artifactId>
<executions>
<execution>
- <id>docker-image-build</id>
+ <id>docker:build</id>
<goals>
<goal>build</goal>
</goals>
- <configuration>
- <tag>${project.version}</tag>
- <buildArgs>
- <GREMLIN_SERVER_DIR>target/apache-tinkerpop-${project.artifactId}-${project.version}-standalone</GREMLIN_SERVER_DIR>
- </buildArgs>
- </configuration>
- </execution>
- <execution>
- <id>docker-image-tag-minor-version</id>
- <goals>
- <goal>tag</goal>
- </goals>
- <configuration>
- <tag>${parsedVersion.majorVersion}.${parsedVersion.minorVersion}</tag>
- <skip>${only.when.is.prerelease.version}</skip>
- </configuration>
+ <phase>package</phase>
</execution>
<execution>
- <id>docker-image-push</id>
- <phase>deploy</phase>
+ <id>docker:push</id>
<goals>
<goal>push</goal>
</goals>
- <configuration>
- <tag>${project.version}</tag>
- <skip>${only.when.is.snapshot.used}</skip>
- </configuration>
- </execution>
- <execution>
- <id>docker-image-push-minor-version</id>
<phase>deploy</phase>
- <goals>
- <goal>push</goal>
- </goals>
- <configuration>
- <tag>${parsedVersion.majorVersion}.${parsedVersion.minorVersion}</tag>
- <skip>${only.when.is.prerelease.version}</skip>
- </configuration>
</execution>
</executions>
- <configuration>
- <repository>tinkerpop/gremlin-server</repository>
- </configuration>
</plugin>
</plugins>
</build>
</profile>
+
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] Cole-Greer commented on pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#issuecomment-1518442805
@lyndonbauto
> So does this now support building on x64 and aarch64?
As of TinkerPop 3.5.5/3.6.3 both x64 and aarch64 were able to build and run TinkerPop out of the box.
> Also does it support cross platform docker image building or just building for the given machines architecture?
The new docker plugin introduced here uses docker buildx under the hood. In theory a user can run this from any platform with docker installed and build an image for any other platform they want. My expectation is that we could build images for all architectures supported by the [alpine base image](https://hub.docker.com/_/alpine/tags) simply by adding them to the `<docker.platforms>` list. The only reason the list is currently limited to x86 and arm is that is all I am able to test. Also in my testing the ARM console image was unstable on my ARM Mac so I opted not to include it in the deployment here. I'm currently unsure what's causing this instability as the console runs fine on ARM without docker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] kenhuuu commented on pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#issuecomment-1513657376
VOTE +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] lyndonbauto commented on pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "lyndonbauto (via GitHub)" <gi...@apache.org>.
lyndonbauto commented on PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#issuecomment-1518478482
@Cole-Greer Sweet, that's really cool mean.
VOTE+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] Cole-Greer commented on a diff in pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#discussion_r1163057037
##########
pom.xml:
##########
@@ -345,6 +345,55 @@ limitations under the License.
<failIfNoMatch>false</failIfNoMatch>
</configuration>
</execution>
+ <execution>
+ <!-- sets the true.when.is.prerelease.version property to true if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-is-prerelease-version2</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>true.when.is.prerelease.version</name>
+ <value>${only.when.is.prerelease.version}</value>
+ <regex>${project.version}</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.majorVersion.minorVersion property to
+ ${parsedVersion.majorVersion}.${parsedVersion.minorVersion} if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-set-docker-version-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.majorVersion.minorVersion</name>
+ <value>${parsedVersion.majorVersion}.${parsedVersion.minorVersion}${true.when.is.prerelease.version}</value>
+ <regex>.*true</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.latest property to "latest" if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-set-docker-latest-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.latest</name>
Review Comment:
This is true but to the best of my knowledge not a new issue. I haven't been able to run the old plugin in my environment so I can't verify this with 100% certainty but `latest` is the default tag for the old plugin and my understanding is that a new `latest` tag gets pushed with every deploy. I believe that currently we are reliant on the fact that typically 3.5.x gets deployed first, and is immediately followed by 3.6.x. I'm not aware of any way to resolve this automatically as there is no awareness within maven if the current deployment is for the "old branch" (3.5) and should not have a latest tag, or if it's for the "new branch" (3.6) and needs a latest tag.
I implemented it this way in order to best replicate the old behaviour. Perhaps it would be better to always have the latest tag disabled unless the release manager sets something like `-Ddocker.latest` during the deployment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] lyndonbauto commented on pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "lyndonbauto (via GitHub)" <gi...@apache.org>.
lyndonbauto commented on PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#issuecomment-1518426644
Made a couple nits, feel free to commit or ignore them.
So does this now support building on x64 and aarch64?
Also does it support cross platform docker image building or just building for the given machines architecture?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] Cole-Greer commented on pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#issuecomment-1515052170
> Is it working for you when you change the version to a non-prerelease one?
@FlorianHockmann That is strange. The tags were working as expected in my testing before submitting the PR. More investigation and testing is needed, I have converted this back to a draft PR for the meantime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on a diff in pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "FlorianHockmann (via GitHub)" <gi...@apache.org>.
FlorianHockmann commented on code in PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#discussion_r1171052912
##########
pom.xml:
##########
@@ -345,6 +345,55 @@ limitations under the License.
<failIfNoMatch>false</failIfNoMatch>
</configuration>
</execution>
+ <execution>
+ <!-- sets the true.when.is.prerelease.version property to true if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-is-prerelease-version2</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>true.when.is.prerelease.version</name>
+ <value>${only.when.is.prerelease.version}</value>
+ <regex>${project.version}</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.majorVersion.minorVersion property to
+ ${parsedVersion.majorVersion}.${parsedVersion.minorVersion} if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-set-docker-version-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.majorVersion.minorVersion</name>
+ <value>${parsedVersion.majorVersion}.${parsedVersion.minorVersion}${true.when.is.prerelease.version}</value>
+ <regex>.*true</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.latest property to "latest" if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-set-docker-latest-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.latest</name>
Review Comment:
I think commenting out makes sense as it clearly documents this on all branches :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] xiazcy merged pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy merged PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on a diff in pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "FlorianHockmann (via GitHub)" <gi...@apache.org>.
FlorianHockmann commented on code in PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#discussion_r1173374302
##########
pom.xml:
##########
@@ -345,11 +345,62 @@ limitations under the License.
<failIfNoMatch>false</failIfNoMatch>
</configuration>
</execution>
+ <execution>
+ <!-- sets the true.when.is.prerelease.version property to true if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-is-prerelease-version2</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>true.when.is.prerelease.version</name>
+ <value>${only.when.is.prerelease.version}</value>
+ <regex>${project.version}</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.majorVersion.minorVersion property to
+ ${parsedVersion.majorVersion}.${parsedVersion.minorVersion} unless a prerelease version was,
+ used. If a prerelease is used, an empty string is assigned instead. -->
+ <id>build-helper-regex-set-docker-version-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.majorVersion.minorVersion</name>
+ <value>${parsedVersion.majorVersion}.${parsedVersion.minorVersion}${true.when.is.prerelease.version}</value>
+ <regex>.*true</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+
+<!-- <execution>-->
+<!-- Excluded as latest tag is currently only updated for the 3.6.x branch -->
+ <!-- sets the docker.tags.latest property to "latest" unless a prerelease version was,
+ used. If a prerelease is used, an empty string is assigned instead. -->
+<!-- <id>build-helper-regex-set-docker-latest-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.latest</name>
+ <value>latest${true.when.is.prerelease.version}</value>
+ <regex>.*true</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution> -->
<execution>
<id>parse-version</id>
<goals>
<goal>parse-version</goal>
- </goals>
+ </goals>
Review Comment:
(nitpick) Wrong indentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] Cole-Greer commented on pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#issuecomment-1516940988
@FlorianHockmann Looks like I accidentally removed too much when I attempted to remove the `latest` tag from 3.5 which was the cause of that error you observed. I have fixed that and run several tests deploying images with different versions to a personal dockerhub repo. You can see all of the tags here (https://hub.docker.com/repository/docker/colegreer/gremlin-console/tags?page=1&ordering=last_updated, https://hub.docker.com/repository/docker/colegreer/gremlin-server/tags?page=1&ordering=last_updated).
I tested deployments of `3.5.6-SNAPSHOT`, `3.5.7`, `3.6.3-SNAPSHOT`, and `3.6.4` to ensure we are getting the correct tags for all of them.
When running these tests I temporarily moved the `docker:push` execution from the `deploy` phase to the `install` phase as I wanted to test docker deployment without triggering any of the other deploy phase goals. It's also noteworthy that the new plugin does not attempt to build the full multi-arch image during the build goal. The reasoning for this being that multi-arch images are much larger in size and slow to build. The full multi-arch image is not built until the `push` goal is executed. This isn't ideal for us but there doesn't appear to be a good workaround with this plugin. Ultimately I don't think it is a big issue though, as we only need the multi-arch images during deployment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on a diff in pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "FlorianHockmann (via GitHub)" <gi...@apache.org>.
FlorianHockmann commented on code in PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#discussion_r1163682374
##########
pom.xml:
##########
@@ -345,6 +345,55 @@ limitations under the License.
<failIfNoMatch>false</failIfNoMatch>
</configuration>
</execution>
+ <execution>
+ <!-- sets the true.when.is.prerelease.version property to true if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-is-prerelease-version2</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>true.when.is.prerelease.version</name>
+ <value>${only.when.is.prerelease.version}</value>
+ <regex>${project.version}</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.majorVersion.minorVersion property to
+ ${parsedVersion.majorVersion}.${parsedVersion.minorVersion} if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-set-docker-version-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.majorVersion.minorVersion</name>
+ <value>${parsedVersion.majorVersion}.${parsedVersion.minorVersion}${true.when.is.prerelease.version}</value>
+ <regex>.*true</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.latest property to "latest" if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-set-docker-latest-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.latest</name>
Review Comment:
Yes, there is no way to handle this automatically. That is why we only have the logic to tag `latest` present on the `3.6-dev` branch and on `master` right now, but not on the `3.5-dev` branch. That way, we can't accidentally tag a Docker image built on `3.5-dev` as `latest`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] Cole-Greer commented on a diff in pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#discussion_r1163729269
##########
pom.xml:
##########
@@ -345,6 +345,55 @@ limitations under the License.
<failIfNoMatch>false</failIfNoMatch>
</configuration>
</execution>
+ <execution>
+ <!-- sets the true.when.is.prerelease.version property to true if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-is-prerelease-version2</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>true.when.is.prerelease.version</name>
+ <value>${only.when.is.prerelease.version}</value>
+ <regex>${project.version}</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.majorVersion.minorVersion property to
+ ${parsedVersion.majorVersion}.${parsedVersion.minorVersion} if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-set-docker-version-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.majorVersion.minorVersion</name>
+ <value>${parsedVersion.majorVersion}.${parsedVersion.minorVersion}${true.when.is.prerelease.version}</value>
+ <regex>.*true</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.latest property to "latest" if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-set-docker-latest-tag</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>docker.tags.latest</name>
Review Comment:
Ahh I wasn't paying close attention to the differences between branches and missed that. I will comment out the `latest` tag piece for `3.5-dev`, and it can be un-commented when it gets merged up.
Alternatively I can remove it entirely here and submit an additional PR for 3.6-dev.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] Cole-Greer commented on a diff in pull request #2007: TINKERPOP-2852 Deploying multi-arch docker images
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #2007:
URL: https://github.com/apache/tinkerpop/pull/2007#discussion_r1163069019
##########
pom.xml:
##########
@@ -345,6 +345,55 @@ limitations under the License.
<failIfNoMatch>false</failIfNoMatch>
</configuration>
</execution>
+ <execution>
+ <!-- sets the true.when.is.prerelease.version property to true if a prerelease version was used,
+ empty string otherwise -->
+ <id>build-helper-regex-is-prerelease-version2</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>regex-property</goal>
+ </goals>
+ <configuration>
+ <name>true.when.is.prerelease.version</name>
+ <value>${only.when.is.prerelease.version}</value>
+ <regex>${project.version}</regex>
+ <replacement></replacement>
+ <failIfNoMatch>false</failIfNoMatch>
+ </configuration>
+ </execution>
+ <execution>
+ <!-- sets the docker.tags.majorVersion.minorVersion property to
Review Comment:
Thanks for catching that. I have fixed the comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org