Details

    • Type: New Feature
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.1.0
    • Fix Version/s: 1.2.0
    • Component/s: deployment
    • Labels:

      Description

      We made a Bigtop/Juju announcement on the dev list last week [0] and have been hacking away to address issues that surfaced during the discussion.

      We're ready to move forward with the proposal to include Juju charm, bundle, and "cloud weather report" sources in the Bigtop repository.

      Commits and an updated PR are incoming... Thanks!

      [0] dev list announce: http://goo.gl/Cna4hJ

        Issue Links

          Activity

          Hide
          cos Konstantin Boudnik added a comment -

          With all subtasks resolved, I am closing this one as well. Thanks!

          Show
          cos Konstantin Boudnik added a comment - With all subtasks resolved, I am closing this one as well. Thanks!
          Hide
          cos Konstantin Boudnik added a comment -

          This is committed and pushed, thanks Kevin W Monroe. I will close the ticket once the subtasks are done with

          Show
          cos Konstantin Boudnik added a comment - This is committed and pushed, thanks Kevin W Monroe . I will close the ticket once the subtasks are done with
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user kwmonroe closed the pull request at:

          https://github.com/apache/bigtop/pull/108

          Show
          githubbot ASF GitHub Bot added a comment - Github user kwmonroe closed the pull request at: https://github.com/apache/bigtop/pull/108
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user kwmonroe commented on the issue:

          https://github.com/apache/bigtop/pull/108

          Thanks c0s! https://github.com/apache/bigtop/commit/d639645e7726577e5898f5da3722c6be65b2c9bd has the goods. Closing this out.

          Show
          githubbot ASF GitHub Bot added a comment - Github user kwmonroe commented on the issue: https://github.com/apache/bigtop/pull/108 Thanks c0s! https://github.com/apache/bigtop/commit/d639645e7726577e5898f5da3722c6be65b2c9bd has the goods. Closing this out.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user c0s commented on the issue:

          https://github.com/apache/bigtop/pull/108

          Committed and pushed to the Apache tree. Please feel free to close this PR

          Show
          githubbot ASF GitHub Bot added a comment - Github user c0s commented on the issue: https://github.com/apache/bigtop/pull/108 Committed and pushed to the Apache tree. Please feel free to close this PR
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user c0s commented on the issue:

          https://github.com/apache/bigtop/pull/108

          Thanks for getting out of the way to address the community feedback, guys.
          I believe in this shape, the patch stands to be as small as it possibly can be. I don't really have anything to say about it, so I will just go ahead and commit it. Thanks!

          Show
          githubbot ASF GitHub Bot added a comment - Github user c0s commented on the issue: https://github.com/apache/bigtop/pull/108 Thanks for getting out of the way to address the community feedback, guys. I believe in this shape, the patch stands to be as small as it possibly can be. I don't really have anything to say about it, so I will just go ahead and commit it. Thanks!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user kwmonroe commented on the issue:

          https://github.com/apache/bigtop/pull/108

          In an effort to reduce the size of this request, I have pulled out the Juju bundles (bigtop-deploy/juju) and Cloud Weather Report (bigtop-tests/cloud-weather-report). This just leaves the core set of hadoop charms up for review.

          Show
          githubbot ASF GitHub Bot added a comment - Github user kwmonroe commented on the issue: https://github.com/apache/bigtop/pull/108 In an effort to reduce the size of this request, I have pulled out the Juju bundles (bigtop-deploy/juju) and Cloud Weather Report (bigtop-tests/cloud-weather-report). This just leaves the core set of hadoop charms up for review.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user johnsca commented on the pull request:

          https://github.com/apache/bigtop/pull/108

          4 spaces is standard for Python code (it is strongly recommended in PEP-8, the official code style guideline for Python), and Python linters will complain if you use something else without adding explicit overrides. If you don't feel strongly about the consistency, I'd recommend keeping 4 spaces for the Python code.

          Show
          githubbot ASF GitHub Bot added a comment - Github user johnsca commented on the pull request: https://github.com/apache/bigtop/pull/108 4 spaces is standard for Python code (it is strongly recommended in PEP-8, the official code style guideline for Python), and Python linters will complain if you use something else without adding explicit overrides. If you don't feel strongly about the consistency, I'd recommend keeping 4 spaces for the Python code.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user c0s commented on the pull request:

          https://github.com/apache/bigtop/pull/108

          I have reviewed the [dev@ thread discussion](https://lists.apache.org/thread.html/Z2uoq25segpbsm6) on this issue and while I agree with Olaf, that commit is a bit on the large side, it seems to be doing all the correct things. I believe the directory layout also makes sense. And the patch is passes the RAT too.

          It might makes sense to add weather-report part separately, but after all it is just one tiny file and a README in there, so it doesn't make that much difference.

          The only real comment I can make is about 2 vs 4 spaces of the indentation as we are using the former. However, the majority of the new code is Python, so I am not really sure if 2-spaces indents look good (or even are acceptable) in that language.

          Hence, I am all +1 on getting this committed, unless there are hard objections from the rest of the community?

          Show
          githubbot ASF GitHub Bot added a comment - Github user c0s commented on the pull request: https://github.com/apache/bigtop/pull/108 I have reviewed the [dev@ thread discussion] ( https://lists.apache.org/thread.html/Z2uoq25segpbsm6 ) on this issue and while I agree with Olaf, that commit is a bit on the large side, it seems to be doing all the correct things. I believe the directory layout also makes sense. And the patch is passes the RAT too. It might makes sense to add weather-report part separately, but after all it is just one tiny file and a README in there, so it doesn't make that much difference. The only real comment I can make is about 2 vs 4 spaces of the indentation as we are using the former. However, the majority of the new code is Python, so I am not really sure if 2-spaces indents look good (or even are acceptable) in that language. Hence, I am all +1 on getting this committed, unless there are hard objections from the rest of the community?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user c0s commented on the pull request:

          https://github.com/apache/bigtop/pull/108#issuecomment-220157012

          Let's see if now it gets linked to the JIRA

          Show
          githubbot ASF GitHub Bot added a comment - Github user c0s commented on the pull request: https://github.com/apache/bigtop/pull/108#issuecomment-220157012 Let's see if now it gets linked to the JIRA

            People

            • Assignee:
              kwmonroe Kevin W Monroe
              Reporter:
              kwmonroe Kevin W Monroe
            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 1h
                1h
                Remaining:
                Remaining Estimate - 1h
                1h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development