You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bigtop.apache.org by eoriented <gi...@git.apache.org> on 2015/07/31 06:10:18 UTC
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
GitHub user eoriented opened a pull request:
https://github.com/apache/bigtop/pull/24
BIGTOP-1126. Add Hama to Bigtop
Hi,
I have modified the patch file to run latest Hama version on bigtop.
Also, I already have tested deb file on Ubuntu 14.04 and rpm file on CentOS 7.
I make sure that Hama was working correctly in local mode.
Please review it.
Thanks.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/eoriented/bigtop hama
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/bigtop/pull/24.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #24
----
commit 39db6111faa37e6904d5d526c4ec26ffdab95fce
Author: Minho Kim <mi...@samsung.com>
Date: 2015-07-31T02:16:05Z
BIGTOP-1126. Add Hama to Bigtop
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by eoriented <gi...@git.apache.org>.
Github user eoriented commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-129699287
Thanks @youngwookim ,
I added my name and edward name in https://github.com/apache/bigtop/blob/master/MAINTAINERS.txt.
However I cannot change assignee to me for [BIGTOP-1126][https://issues.apache.org/jira/browse/BIGTOP-1126]. I have no permit for Bigtop's Jira.
How can I get authority changing assignee in this issue?
Please check code modified and let me know above question.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-129729375
OK. You should 'rebase' your working branch to ASF master before sending a PR because your commit is far behind current master branch. Sometimes there are merge conflicts while someone is trying to merge the patch. This is a simple workflow for 'git rebase':
```
git checkout master
git pull asf master
git checkout YOUR-WORKING-BRANCH
git rebase -i asf/master
git push origin YOUR-WORKING-BRANCH
```
'asf' is a name for ASF git remote or github's bigtop mirror!
For more details, just googling 'git rebase'!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by jayunit100 <gi...@git.apache.org>.
Github user jayunit100 commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-126661034
Why are there two commits ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-129703865
Thank you, @eoriented ! I've left a message to our moderator. Will be changed the assignee field on JIRA.
In the meanwhile, I'll test your patch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-129737677
@eoriented Please hold off the work that I've mentioned. It looks like a bug on Github while github is generating the diff or patch. :-( Will investigate it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-126566044
Thanks for your PR @eoriented but I'm afraid that you've missed some related files: packaging, smokes and etc. Please make sure the current PR has all your local changes. IMO, it looks like the PR has only changes on bigtop BOM(bigtop.mk)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/24#discussion_r36049839
--- Diff: bigtop-packages/src/common/hama/do-component-build ---
@@ -0,0 +1,19 @@
+#!/bin/sh
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -ex
+
+mvn clean install -DskipTests "$@"
--- End diff --
IMO, It would be nice to have appropriate dependency here:
{code}
-mvn clean install -DskipTests "$@"
+. `dirname ${0}`/bigtop.bom
+
+mvn -Phadoop2 -Dhadoop.version=$HADOOP_VERSION -Dzookeeper.version=$ZOOKEEPER_VERSION clean install -DskipTests "$@"
{code}
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-129745248
Got it, It's not a bug, Github's 'patch' is not same as 'diff'. I did not know that! :-( I think fixing the commit history is not easy at this point so I would encourage you to make new PR or patch from the current diff -- https://github.com/apache/bigtop/pull/24.diff
First, please make a fresh branch and then apply your changes into the branch with a same commit message. like below:
1. Download current diff from github:
```
cd /tmp/
wget https://github.com/apache/bigtop/pull/24.diff
```
2. Apply the diff (again):
```
git pull asf master
git checkout -b BIGTOP-1126
git apply /tmp/24.diff
git add -A
git commit -m 'BIGTOP-1126. ...'
git push origin BIGTOP-1126
```
And then, please re-send a PR. Maybe... there are better ways but don't know exactly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by eoriented <gi...@git.apache.org>.
Github user eoriented commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-128005486
Thank you for your opinion.
I modified code, which reflects your comment.
Please check it and let me know if more modification.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/24#discussion_r36049924
--- Diff: bigtop-packages/src/deb/hama/control ---
@@ -0,0 +1,67 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+Source: hama
+Section: misc
+Priority: extra
+Maintainer: BigTop
+Build-Depends: debhelper (>= 6)
--- End diff --
Please update compat to 9 [1]
[1] https://cwiki.apache.org/confluence/display/BIGTOP/Bigtop+Packaging
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by eoriented <gi...@git.apache.org>.
Github user eoriented commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-126566723
Oops,
I'm sorry that I forget committing packaging files.
I'll commit now.
Thanks @youngwookim
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by eoriented <gi...@git.apache.org>.
Github user eoriented commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-129738666
@youngwookim Ok. I'll wait it.
Thank you for a lucid explanation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by eoriented <gi...@git.apache.org>.
Github user eoriented commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-129723106
@youngwookim I'm sorry that I'm not used to using git.
Is it right? or If it's wrong, please let me know how to do that.
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-127085503
@eoriented, Overall looks good to me. Left a few comments. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by eoriented <gi...@git.apache.org>.
Github user eoriented closed the pull request at:
https://github.com/apache/bigtop/pull/24
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-128229019
Thanks @eoriented, Almost there!
A few things:
- Please add a following line to the spec file
```
Source6: bigtop.bom
```
- Please add an entry for Hama at MAINTAINERS, https://github.com/apache/bigtop/blob/master/MAINTAINERS.txt For now, an entry for MAINTAINERS.txt is not mandatory but I believe that it would be good to have maintainers to prevent the orphaned component and to resolve issues from individual component quickly.
- Current assignee for https://issues.apache.org/jira/browse/BIGTOP-1126 is @edwardyoon This should be changed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-129752689
@eoriented Please keep in mind you should have one commit and it's rebased to current master! Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by eoriented <gi...@git.apache.org>.
Github user eoriented commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-126854536
I changed one commit.
Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by edwardyoon <gi...@git.apache.org>.
Github user edwardyoon commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-128186607
+1 lgtm.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/24#discussion_r36049933
--- Diff: bigtop-packages/src/deb/hama/control ---
@@ -0,0 +1,67 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+Source: hama
+Section: misc
+Priority: extra
+Maintainer: BigTop
+Build-Depends: debhelper (>= 6)
+Standards-Version: 3.8.0
--- End diff --
Please Standards-Version to 3.9.4 [1]
[1] https://cwiki.apache.org/confluence/display/BIGTOP/Bigtop+Packaging
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-129711957
@eoriented I got a merge conflict from 'MAINTAINERS.txt' Please rebase your changes to master branch. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by edwardyoon <gi...@git.apache.org>.
Github user edwardyoon commented on the pull request:
https://github.com/apache/bigtop/pull/24#issuecomment-128231569
@eoriented Please add your name and my asf id: edwardyoon
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bigtop pull request: BIGTOP-1126. Add Hama to Bigtop
Posted by youngwookim <gi...@git.apache.org>.
Github user youngwookim commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/24#discussion_r36061325
--- Diff: bigtop-packages/src/deb/hama/compat ---
@@ -0,0 +1 @@
+6
--- End diff --
Forgot to mention, please update compat and debhelper to 9. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---