You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2021/06/14 07:20:30 UTC

[GitHub] [beam] emilymye commented on a change in pull request #14990: [BEAM-4767] Remove release script lines to replace container version

emilymye commented on a change in pull request #14990:
URL: https://github.com/apache/beam/pull/14990#discussion_r650203715



##########
File path: release/src/main/scripts/set_version.sh
##########
@@ -76,9 +76,6 @@ if [[ -z "$IS_SNAPSHOT_VERSION" ]] ; then
   sed -i -e "s/^__version__ = .*/__version__ = '${TARGET_VERSION}'/" sdks/python/apache_beam/version.py
   sed -i -e "s/sdk_version=.*/sdk_version=$TARGET_VERSION/" gradle.properties
   sed -i -e "s/SdkVersion = .*/SdkVersion = \"$TARGET_VERSION\"/" sdks/go/pkg/beam/core/core.go
-  # TODO: [BEAM-4767]
-  sed -i -e "s/'dataflow.fnapi_container_version' : .*/'dataflow.fnapi_container_version' : '${TARGET_VERSION}'/" runners/google-cloud-dataflow-java/build.gradle
-  sed -i -e "s/'dataflow.legacy_container_version' : .*/'dataflow.legacy_container_version' : 'beam-${TARGET_VERSION}'/" runners/google-cloud-dataflow-java/build.gradle

Review comment:
       Because we switched DataflowRunner to choose the tag based on SDK version: https://github.com/apache/beam/blob/4a78a81f1e9f2f9f73eda34c9eb5651eb9dad885/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java#L2221-L2227
   
   we in general shouldn't have to do these replacements anymore at all. 
   
   The RC should have default image tag = SDK version/$TARGET_VERSION set in earlier lines here. This behavior matches current behavior where Dataflow GCR image RCs are pushed at 2.XX.0, not 2.XX.0_rcX (i.e. only DockerHub images are pushed at _rc tag). If we wanted to make image tags with the RC, there are other changes (dev SDK check, container release/cloning) that would need to happen before these replacements lines would do anything.

##########
File path: release/src/main/scripts/set_version.sh
##########
@@ -76,9 +76,6 @@ if [[ -z "$IS_SNAPSHOT_VERSION" ]] ; then
   sed -i -e "s/^__version__ = .*/__version__ = '${TARGET_VERSION}'/" sdks/python/apache_beam/version.py
   sed -i -e "s/sdk_version=.*/sdk_version=$TARGET_VERSION/" gradle.properties
   sed -i -e "s/SdkVersion = .*/SdkVersion = \"$TARGET_VERSION\"/" sdks/go/pkg/beam/core/core.go
-  # TODO: [BEAM-4767]
-  sed -i -e "s/'dataflow.fnapi_container_version' : .*/'dataflow.fnapi_container_version' : '${TARGET_VERSION}'/" runners/google-cloud-dataflow-java/build.gradle
-  sed -i -e "s/'dataflow.legacy_container_version' : .*/'dataflow.legacy_container_version' : 'beam-${TARGET_VERSION}'/" runners/google-cloud-dataflow-java/build.gradle

Review comment:
       Because we switched DataflowRunner to choose the tag based on SDK version: https://github.com/apache/beam/blob/4a78a81f1e9f2f9f73eda34c9eb5651eb9dad885/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java#L2221-L2227
   
   we shouldn't have to do these replacements anymore at all - the values of these properties should only matter when the SDK is using a beam-master-YYYYMMDD container. 
   
   The RC should have default image tag = SDK version/$TARGET_VERSION set in earlier lines here. This behavior matches current behavior where Dataflow GCR image RCs are pushed at 2.XX.0, not 2.XX.0_rcX (i.e. only DockerHub images are pushed at _rc tag). If we wanted to make image tags with the RC, there are other changes (dev SDK check, container release/cloning) that would need to happen before these replacements lines would do anything.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org