Skip to content
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

[ZEPPELIN-986] Create publish release script #994

Closed
wants to merge 12 commits into from

Conversation

minahlee
Copy link
Member

@minahlee minahlee commented Jun 10, 2016

What is this PR for?

This PR is to automate release publish to maven repository.
We used to use maven-deploy-plugin and maven-release-plugin for release but somehow it didn't work well with Zeppelin so 0.5.5 and 0.5.6 haven't been published to maven repository.

Publishing release to maven repository will eventually help zeppelin to reduce binary package size by leading users to use Dynamic interpreter loading(#908).
Originally below modules were skipped for maven release

  • all interpreters(except spark)
  • zeppelin-display
  • zeppelin-server
  • zeppelin-distribution

on the other hand this pr will skip only:

  • zeppelin-distribution

What type of PR is it?

Infra

Todos

  • Include SparkR/R interpreter in release
  • Create common_release.sh to remove build configuration duplication
  • Check curl networking failure

What is the Jira issue?

ZEPPELIN-986

Questions:

@minahlee minahlee changed the title [WIP][ZEPPELIN-986] Create publish release script [ZEPPELIN-986] Create publish release script Jun 10, 2016
@minahlee minahlee changed the title [ZEPPELIN-986] Create publish release script [WIP][ZEPPELIN-986] Create publish release script Jun 10, 2016
@Leemoonsoo
Copy link
Member

CI failiure is unrelated. Looks good to me

@minahlee minahlee closed this Jun 13, 2016
@minahlee minahlee reopened this Jun 13, 2016
@minahlee minahlee changed the title [WIP][ZEPPELIN-986] Create publish release script [ZEPPELIN-986] Create publish release script Jun 13, 2016

# Using Nexus API documented here:
# https://support.sonatype.com/hc/en-us/articles/213465868-Uploading-to-a-Staging-Repository-via-REST-API
echo "Creating Nexus staging repository"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall this be extracted to a function, how do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make it as function, this function should return value staged_repo_id which is not supported in bash. I know there is tricky way to handle it, but not sure how beneficial it would be since this will be used only once in a script. What do you think?

@bzz
Copy link
Member

bzz commented Jun 14, 2016

Looks great, I think there are just few things left:

@minahlee if we want to remove maven release plugin, could you also point to the place that does automate the version switch in POM files (as AFAIK that is the only purpose we used that plugin so far)

cd "${WORKING_DIR}/zeppelin"

# Force release version
mvn versions:set -DnewVersion="${RELEASE_VERSION}"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bzz this is where pom.xml is changed :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing it out!

@minahlee
Copy link
Member Author

@bzz Thank you for the review, I handled style in 21ca610. Please let me know if you have any other suggestions :)

@echarles
Copy link
Member

why spark is not published. is it related to the assembly?

Btw, spark 2 doesn't use assembly any more and favor dependencies in separate jars. See https://issues.apache.org/jira/browse/SPARK-11157 for more details.

file_short="$(echo "${file}" | sed -e 's/\.\///')"
dest_url="${nexus_upload}/org/apache/zeppelin/$file_short"
echo " Uploading ${file_short}"
curl -u "${ASF_USERID}:${ASF_PASSWORD}" --upload-file "${file_short}" "${dest_url}"
Copy link
Member

@bzz bzz Jun 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we do not use -e flag for this scripts (which sounds good to me), shall not we check return of, at least, network-related commands?

This way we could make sure that if it have failed due to networking reasons, we print to the user meaningful error conditions and let him then re-run the script?

@bzz
Copy link
Member

bzz commented Jun 15, 2016

@minahlee Great, I think if checking error conditions could be added to network-related commands - it would be even more awesome, as this is quite common failure pattern.

Other then that, looks great to me and thank you for automating this.

@echarles Looking at this line I was under impression that it's only zeppelin-server and zeppelin-distribution that is not published here..

As 2 release scripts share the same lines of build setup - it can be error-prone to modify it in both places, in case something needs to be added to release.

@minahlee shall we keep it in the file and read from there in both scripts, or shall we define it in third one common_release.sh and source it from two others?

@minahlee
Copy link
Member Author

@echarles I think my description caused confusion. Spark will be included in release since most of Zeppelin user is based on Spark. The modules I wrote in description is skipped modules for publish and Spark always hasn't been skipped from release.

@echarles
Copy link
Member

Didn't look at the code... just saw in the description (and @minahlee just confirmed it would be published).

Thinking more about this PR:

  • Why the deploy and release plugins were not working? Using scrips instead of existing plugins is not ideal to me.
  • I understand distribution should not be published, but server should be. It contains the repo components that can be used elsewhere. Apart from the fact we could have a separate module for the repo, all components (excpect distribution) should be published. That's a rule of thumb and if we don't follow it, someone is gonna come sooner or later and ask for the missing bits.

@bzz
Copy link
Member

bzz commented Jun 15, 2016

Thank you for the feedback @echarles !

I think it should be fine to add missing bits, as soon as they are identified and there is somebody with actual use case for it later on. Would you be willing to log it as a JIRA issue?

As person who did release management for Zeppelin before - I had no troubles with release plugin, although calling it multiple times with different arguments, instead of single script was quite error prone.

If a manager for a current release though wants to try another solution and provides a robust one (as in this PR) - it should be fine, especially given that we can always get back to the plugins if desired.

@echarles
Copy link
Member

Good to hear the release plugin is working fine.

The release manager is responsible for the tools he wants. Fine to me if it is a script or a plugin, but at the end the artifacts must find their way to maven central.

For this PR, is it possible to ensure zeppelin-server will be also published?

@minahlee
Copy link
Member Author

minahlee commented Jun 15, 2016

@echarles

  • This script is written exactly because of the reason that @bzz explained above.

calling it multiple times with different arguments, instead of single script was quite error prone.

And for the record, Zeppelin artifacts have not published 0.5.5, 0.5.6 to maven repository, but I am not sure if it is deploy plugin issue or not.

  • AFAIK, repo is under zeppelin-zengine module, not zeppelin-server. But if there is any other use case for zeppelin-server other than notebook repo I don't see the reason not to publish. If you come up with other use case, please let me know

@echarles
Copy link
Member

@minahlee thx for this precision and context.
True, I just realized after sending the comment that repo is not part of zeppelin-server.
However, I need zeppelin-server in my project (I kind off embed the web and websocket server) - Publishing all artifcats is also in my understanding a common practice. WDYT. Is there any issue you face to publish the server?

@minahlee
Copy link
Member Author

@echarles If there are such needs, nothing stops us from publishing it. I will make change accordingly. Thank you for sharing your situation.

@bzz
Copy link
Member

bzz commented Jun 16, 2016

@minahlee Great!

I think it should be ready to merge as soon as issues highlighted above

a. failing networking curl
b. build configuration duplication

are addressed

@minahlee
Copy link
Member Author

@bzz Sorry for late response, I am working on it and going to push commits by today. Thanks for heads up :)

@bzz
Copy link
Member

bzz commented Jun 20, 2016

Looks awesome to me, thank you @minahlee !

2 minor things are a bit confusing:

  • the purpose of dev/publish_release.sh.hehe file

  • some artifacts (namely, integration for Geode and Scalding) we publish in maven, but AFAIK they are not part of the convenience binary for the release

    publish: -Pspark-1.6 -Phadoop-2.4 -Pyarn -Ppyspark -Psparkr -Pr -Pgeode -Pscalding
    release: -Pspark-1.6 -Phadoop-2.4 -Pyarn -Ppyspark -Psparkr -Pr
    

@minahlee
Copy link
Member Author

minahlee commented Jun 20, 2016

@bzz Thanks for the review. I removed mistakenly pushed file.
I included geode, scalding interpreter in maven publish script so that geode, scalding users can take advantage of #1042.
To be honest, I don't know why these two interpreters were not included in last release. It would be appreciated if someone can clarify it.

@Leemoonsoo
Copy link
Member

@minahlee Here's related issues why geode and scalding is not in maven publish script.

Geode - https://issues.apache.org/jira/browse/ZEPPELIN-375
Scalding - https://issues.apache.org/jira/browse/ZEPPELIN-972

Please take a look. To include them, bin_license/LICENSE file need to be updated with their transitive dependencies(geode, scalding interpreter), and remove 3rd party repository (scalding interpreter)

@minahlee
Copy link
Member Author

@Leemoonsoo Thanks for pointing it out! I reverted commit of adding geode, scalding

@bzz
Copy link
Member

bzz commented Jun 21, 2016

Got it, thank for reminding! Let's merge this guy then.

Any 👍 or 👎 ?

@minahlee
Copy link
Member Author

Merge if there is no more discussion

@asfgit asfgit closed this in 85d7057 Jun 23, 2016
asfgit pushed a commit that referenced this pull request Jun 23, 2016
### What is this PR for?
This PR is to automate release publish to maven repository.
We used to use maven-deploy-plugin and maven-release-plugin for release but somehow it didn't work well with Zeppelin so 0.5.5 and 0.5.6 haven't been published to maven repository.

Publishing release to maven repository will eventually help zeppelin to reduce binary package size by leading users to use Dynamic interpreter loading(#908).
Originally below modules were skipped for maven release
 - all interpreters(except spark)
 - zeppelin-display
 - zeppelin-server
 - zeppelin-distribution

on the other hand this pr will skip only:
 - zeppelin-distribution

### What type of PR is it?
Infra

### Todos
- [x] Include SparkR/R interpreter in release
- [x] Create common_release.sh to remove build configuration duplication
- [x] Check curl networking failure

### What is the Jira issue?
[ZEPPELIN-986](https://issues.apache.org/jira/browse/ZEPPELIN-986)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? Yes, https://cwiki.apache.org/confluence/display/ZEPPELIN/Preparing+Zeppelin+Release will be updated accordingly once this pr is merged.

Author: Mina Lee <minalee@apache.org>

Closes #994 from minahlee/ZEPPELIN-986 and squashes the following commits:

b0e8e67 [Mina Lee] Revert "Add geode, scalding profile in maven artifact build"
cd4cbcd [Mina Lee] curl failure check
c0ea07c [Mina Lee] Fix wrong indentation
a88bc1d [Mina Lee] Add geode, scalding profile in maven artifact build
2cced61 [Mina Lee] Add r to binary package and maven build
903bc12 [Mina Lee] Move duplicate code to common_release.sh
a3eb676 [Mina Lee] Include zeppelin-server module in publish artifiact
48d338f [Mina Lee] Rollback mistakenly removed plugin
aafaf42 [Mina Lee] Follow google shell  style guide
30dcc65 [Mina Lee] remove deploy plugin from pom since custom script will be used instead for deploy
cd1f08c [Mina Lee] Refactor create release script
e764f5f [Mina Lee] Add maven publish release script

(cherry picked from commit 85d7057)
Signed-off-by: Mina Lee <minalee@apache.org>
@minahlee minahlee deleted the ZEPPELIN-986 branch August 8, 2016 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants