Add pod details page #740

digitalfishpond · 2016-05-16T10:48:17Z

Details page for pods and corresponding tests #650

This change is

floreks · 2016-05-16T11:01:30Z

Looks nice. Just a few comments.

Previously, digitalfishpond (Denis Poisson) wrote…

Add pod details page

Details page for pods and corresponding tests #650

Reviewed 26 of 30 files at r1.
Review status: 22 of 26 files reviewed at latest revision, 5 unresolved discussions.

src/app/backend/resource/deployment/deploymentlist.go, line 22 [r1] (raw file):

  "github.com/kubernetes/dashboard/resource/common"

Try to follow the rule and use only 1 empty line between blocks

src/app/backend/resource/pod/poddetail.go, line 27 [r1] (raw file):

)

// Pod is a presentation layer view of Kubernetes Pod resource. This means

swap/Pod/Pod detail

src/app/backend/resource/pod/poddetail.go, line 53 [r1] (raw file):

}

func GetPodDetail(client k8sClient.Interface, heapsterClient client.HeapsterClient,

Document it please

src/app/frontend/poddetail/poddetail.html, line 22 [r1] (raw file):

      <md-tab label="Overview">
        <kd-pod-info pod="::ctrl.podDetail"></kd-pod-info>

Remove empty line

src/app/frontend/poddetail/podinfo.html, line 38 [r1] (raw file):

    </kd-info-card-entry>
    <kd-info-card-entry title="Images">
      <div ng-repeat="image in $ctrl.pod.containerImages">

swap/$ctrl.pod.containerImages/::$ctrl.pod.containerImages

Comments from Reviewable

digitalfishpond · 2016-05-16T11:18:27Z

Review status: 22 of 25 files reviewed at latest revision, 5 unresolved discussions.

src/app/backend/resource/pod/poddetail.go, line 27 [r1] (raw file):

Previously, floreks (Sebastian Florek) wrote…

swap/Pod/Pod detail

Done.

src/app/backend/resource/pod/poddetail.go, line 53 [r1] (raw file):

Previously, floreks (Sebastian Florek) wrote…

Document it please

Done.

src/app/frontend/poddetail/poddetail.html, line 22 [r1] (raw file):

Previously, floreks (Sebastian Florek) wrote…

Remove empty line

Done.

src/app/frontend/poddetail/podinfo.html, line 38 [r1] (raw file):

Previously, floreks (Sebastian Florek) wrote…

swap/$ctrl.pod.containerImages/::$ctrl.pod.containerImages

Done.

src/app/backend/resource/deployment/deploymentlist.go, line 22 [r1] (raw file):

Previously, floreks (Sebastian Florek) wrote…

Try to follow the rule and use only 1 empty line between blocks

Done.

Comments from Reviewable

digitalfishpond · 2016-05-16T11:18:50Z

PTAL

Previously, floreks (Sebastian Florek) wrote…

Looks nice. Just a few comments.

Review status: 22 of 25 files reviewed at latest revision, 5 unresolved discussions.

Comments from Reviewable

floreks · 2016-05-16T11:25:03Z

Great!

@bryk could you take a look also?

Previously, digitalfishpond (Denis Poisson) wrote…

PTAL

Reviewed 3 of 4 files at r2.
Review status: 24 of 25 files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

bryk · 2016-05-16T13:27:21Z

This does not work on gulp serve:prod. Looks good otherwise. Please fix and let's merge :)
If you need help debugging this, let me know.

Previously, floreks (Sebastian Florek) wrote…

Great! :lgtm:

@bryk could you take a look also?

Reviewed 26 of 30 files at r1, 3 of 4 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

src/test/backend/resource/pod/podcommon_test.go, line 24 [r2] (raw file):

  "github.com/kubernetes/dashboard/resource/common"
  //"app/backend/resource/pod"

Remove?

Comments from Reviewable

digitalfishpond · 2016-05-16T13:41:45Z

@bryk I am not getting any problems with gulp serve:prod... do you have any more clues?

Previously, bryk (Piotr Bryk) wrote…

This does not work on gulp serve:prod. Looks good otherwise. Please fix and let's merge :)
If you need help debugging this, let me know.

Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

bryk · 2016-05-16T13:54:57Z

I just dont see pod details page on serve:prod
I'll check again tomorrow morning, if needed.

Previously, digitalfishpond (Denis Poisson) wrote…

@bryk I am not getting any problems with gulp serve:prod... do you have any more clues?

Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

digitalfishpond · 2016-05-16T13:57:33Z

Ah!, no, you are absolutely right. My bad. I will fix tomorrow morning.

bryk · 2016-05-17T08:43:35Z

Review status: all files reviewed at latest revision, 6 unresolved discussions.

a discussion (no related file):
Btw, this fails if pod has no labels. Can you ng-if this?

    <kd-info-card-entry title="Labels">
      <kd-labels labels="::$ctrl.pod.objectMeta.labels"></kd-labels>
    </kd-info-card-entry>

src/app/frontend/poddetail/poddetail_controller.js, line 22 [r2] (raw file):

   * @param {!backendApi.PodDetail} podDetail
   */
  constructor(podDetail) {

Needs @ngInject

src/app/frontend/poddetail/poddetail_stateconfig.js, line 55 [r2] (raw file):

 * @param {!angular.$resource} $resource
 * @return {!angular.Resource<!backendApi.PodDetail>}
 */

Missing ngInject here

src/app/frontend/poddetail/poddetail_stateconfig.js, line 63 [r2] (raw file):

 * @param {!angular.Resource<!backendApi.PodDetail>} podDetailResource
 * @return {!angular.$q.Promise}
 */

Missing ngInject here

src/app/frontend/podlist/podcardlist_component.js, line 25 [r2] (raw file):

   * @ngInject
   */
  constructor($state) {

Needs @param

Comments from Reviewable

bryk · 2016-05-17T08:46:47Z

Problem solved. See my comments.

Story how I found it. I saw that on scripts:prod there are no errors, and that when I want to see pod details page there is unknown error. This means that UI router ate some exception. This means that this is a problem with it.

Then I added console.log(event, toState, toParams, fromState, fromParams, error); to error_module.js to verify this. I saw 'unknown provider' error which confirmed my suspicions. With this I realized that export function getPodDetailResource($resource, $stateParams) { have no ngInject :)

Have a nice day!

Previously, digitalfishpond (Denis Poisson) wrote…

Ah!, no, you are absolutely right. My bad. I will fix tomorrow morning.

Review status: all files reviewed at latest revision, 5 unresolved discussions.

Comments from Reviewable

digitalfishpond · 2016-05-17T09:47:38Z

Thank you so much for this :) PTAL

Previously, bryk (Piotr Bryk) wrote…

Problem solved. See my comments.

Story how I found it. I saw that on scripts:prod there are no errors, and that when I want to see pod details page there is unknown error. This means that UI router ate some exception. This means that this is a problem with it.

Then I added console.log(event, toState, toParams, fromState, fromParams, error); to error_module.js to verify this. I saw 'unknown provider' error which confirmed my suspicions. With this I realized that export function getPodDetailResource($resource, $stateParams) { have no ngInject :)

Have a nice day!

Review status: 17 of 25 files reviewed at latest revision, 5 unresolved discussions.

a discussion (no related file):

Previously, bryk (Piotr Bryk) wrote…

Btw, this fails if pod has no labels. Can you ng-if this?

    <kd-info-card-entry title="Labels">
      <kd-labels labels="::$ctrl.pod.objectMeta.labels"></kd-labels>
    </kd-info-card-entry>

Done.

src/app/frontend/poddetail/poddetail_controller.js, line 22 [r2] (raw file):

Previously, bryk (Piotr Bryk) wrote…

Needs @ngInject

Done.

src/app/frontend/poddetail/poddetail_stateconfig.js, line 55 [r2] (raw file):

Previously, bryk (Piotr Bryk) wrote…

Missing ngInject here

Done.

src/app/frontend/poddetail/poddetail_stateconfig.js, line 63 [r2] (raw file):

Previously, bryk (Piotr Bryk) wrote…

Missing ngInject here

Done.

src/app/frontend/podlist/podcardlist_component.js, line 25 [r2] (raw file):

Previously, bryk (Piotr Bryk) wrote…

Needs @param

Done.

src/test/backend/resource/pod/podcommon_test.go, line 24 [r2] (raw file):

Previously, bryk (Piotr Bryk) wrote…

Remove?

Done.

Comments from Reviewable

codecov-io · 2016-05-17T09:59:30Z

Current coverage is 94.15%

Merging #740 into master will increase coverage by 0.38%

File ...ereader_directive.js (not in diff) was modified. more
- Misses -6
- Partials 0
- Hits +6

@@             master       #740   diff @@
==========================================
  Files           183        188     +5   
  Lines          1459       1488    +29   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1368       1401    +33   
+ Misses           91         87     -4   
  Partials          0          0

Powered by Codecov. Last updated by d7b0cec...c0c734b

bryk · 2016-05-17T10:12:07Z

Previously, codecov-io wrote…

[Current coverage][cc-pull] is 94.15%

Merging [#740][cc-pull] into [master][cc-base-branch] will increase coverage by 0.38%

File ...ereader_directive.js (not in diff) was modified. more

Misses -6

Partials 0

Hits +6
@@             master       #740   diff @@
==========================================
  Files           183        188     +5   
  Lines          1459       1488    +29   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1368       1401    +33   
+ Misses           91         87     -4   
  Partials          0          0          
Powered by Codecov. Last updated by [d7b0cec...c0c734b][cc-compare]
[cc-base-branch]: https://codecov.io/gh/kubernetes/dashboard/branch/master?src=pr
[cc-compare]: https://codecov.io/gh/kubernetes/dashboard/compare/d7b0cec81c6c2d66796ba14b1f049f25a53fee20...c0c734b2b2ed9bb48142d3a18b64e007fbebaabe
[cc-pull]: https://codecov.io/gh/kubernetes/dashboard/pull/740?src=pr

Reviewed 8 of 8 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

googlebot added the cla: yes label May 16, 2016

digitalfishpond force-pushed the pod-details branch from 1c52d80 to d8d1ee4 Compare May 16, 2016 11:18

Add pod details page

2d29ae5

digitalfishpond force-pushed the pod-details branch from d8d1ee4 to 2d29ae5 Compare May 17, 2016 09:46

bryk merged commit 18dab9f into kubernetes:master May 17, 2016

floreks mentioned this pull request May 20, 2016

Implement details pages for resources #651

Closed

22 tasks

digitalfishpond deleted the pod-details branch May 31, 2016 08:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add pod details page #740

Add pod details page #740

Uh oh!

digitalfishpond commented May 16, 2016 •

edited by bryk

Loading

Uh oh!

floreks commented May 16, 2016

Add pod details page

Uh oh!

digitalfishpond commented May 16, 2016

Uh oh!

digitalfishpond commented May 16, 2016

Uh oh!

floreks commented May 16, 2016

Uh oh!

bryk commented May 16, 2016

Uh oh!

digitalfishpond commented May 16, 2016

Uh oh!

bryk commented May 16, 2016

Uh oh!

digitalfishpond commented May 16, 2016

Uh oh!

bryk commented May 17, 2016

Uh oh!

bryk commented May 17, 2016

Uh oh!

digitalfishpond commented May 17, 2016

Uh oh!

codecov-io commented May 17, 2016

Uh oh!

bryk commented May 17, 2016

[Current coverage][cc-pull] is 94.15%

Uh oh!

Uh oh!

Add pod details page #740

Add pod details page #740

Uh oh!

Conversation

digitalfishpond commented May 16, 2016 • edited by bryk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

floreks commented May 16, 2016

Add pod details page

Uh oh!

digitalfishpond commented May 16, 2016

Uh oh!

digitalfishpond commented May 16, 2016

Uh oh!

floreks commented May 16, 2016

Uh oh!

bryk commented May 16, 2016

Uh oh!

digitalfishpond commented May 16, 2016

Uh oh!

bryk commented May 16, 2016

Uh oh!

digitalfishpond commented May 16, 2016

Uh oh!

bryk commented May 17, 2016

Uh oh!

bryk commented May 17, 2016

Uh oh!

digitalfishpond commented May 17, 2016

Uh oh!

codecov-io commented May 17, 2016

Current coverage is 94.15%

Uh oh!

bryk commented May 17, 2016

[Current coverage][cc-pull] is 94.15%

Uh oh!

Uh oh!

digitalfishpond commented May 16, 2016 •

edited by bryk

Loading