Skip to content

Conversation

maciaszczykm
Copy link
Member

@maciaszczykm maciaszczykm commented Jun 1, 2016

Added Job resource support in the backend.

Connected to #788.


This change is Reviewable

@codecov-io
Copy link

codecov-io commented Jun 1, 2016

Current coverage is 94.86%

Merging #792 into master will increase coverage by 0.37%

  1. File ...ereader_directive.js (not in diff) was modified. more
    • Misses -6
    • Partials 0
    • Hits +6
@@             master       #792   diff @@
==========================================
  Files           197        197          
  Lines          1596       1596          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1508       1514     +6   
+ Misses           88         82     -6   
  Partials          0          0          

Powered by Codecov. Last updated by 39c7d30...3779f1a

@maciaszczykm maciaszczykm mentioned this pull request Jun 1, 2016
2 tasks
@bryk bryk self-assigned this Jun 1, 2016
@bryk
Copy link
Contributor

bryk commented Jun 1, 2016

Looks nice. Add more tests and we're ready to go

Previously, codecov-io wrote…

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

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

  1. File ...ereader_directive.js (not in diff) was modified. more
    • Misses -6
    • Partials 0
    • Hits +6
@@             master       #792   diff @@
==========================================
  Files           187        187          
  Lines          1462       1462          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1382       1388     +6   
+ Misses           80         74     -6   
  Partials          0          0          

Powered by Codecov. Last updated by [25dac01...df91ceb][cc-compare]
[cc-base-branch]: https://codecov.io/gh/kubernetes/dashboard/branch/master?src=pr
[cc-compare]: https://codecov.io/gh/kubernetes/dashboard/compare/25dac017c001b170a4facc67e8b206971e5070e4...df91ceb57dac311028cb7d79a706bfe8adfc6569
[cc-pull]: https://codecov.io/gh/kubernetes/dashboard/pull/792?src=pr


Reviewed 2 of 6 files at r1, 7 of 7 files at r2.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


src/app/backend/resource/common/types.go, line 110 [r2] (raw file):

  Extension bool
}{
  ResourceKindService:               {"services", false},

@floreks We need PetSets here, remember :)


src/app/backend/resource/common/types.go, line 117 [r2] (raw file):

  ResourceKindReplicaSet:            {"replicasets", true},
  ResourceKindDaemonSet:             {"daemonsets", false},
  ResourceKindJob:                   {"job", false},

Should this be true? I think so.


src/app/backend/resource/job/joblist.go, line 35 [r2] (raw file):

// Job is a presentation layer view of Kubernetes Job resource. This means
// it is Job plus additional augumented data we can get from other sources
// (like services that target the same pods).

(like services that target the same pods). <- this is probably to remove :)


Comments from Reviewable

@floreks
Copy link
Member

floreks commented Jun 2, 2016

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


src/app/backend/resource/common/types.go, line 117 [r2] (raw file):

Previously, bryk (Piotr Bryk) wrote…

Should this be true? I think so.

Also I think it should be `jobs`.

Comments from Reviewable

@maciaszczykm
Copy link
Member Author

Review status: 8 of 9 files reviewed at latest revision, 3 unresolved discussions.


src/app/backend/resource/common/types.go, line 117 [r2] (raw file):

Previously, floreks (Sebastian Florek) wrote…

Also it should be jobs i think.

Updated to `"jobs", true`. Still I think extension client won't work, because jobs use batch client.

src/app/backend/resource/job/joblist.go, line 35 [r2] (raw file):

Previously, bryk (Piotr Bryk) wrote…

(like services that target the same pods). <- this is probably to remove :)

Done.

Comments from Reviewable

@floreks
Copy link
Member

floreks commented Jun 2, 2016

Review status: 7 of 9 files reviewed at latest revision, 3 unresolved discussions.


src/app/backend/resource/common/types.go, line 117 [r2] (raw file):

Previously, maciaszczykm (Marcin Maciaszczyk) wrote…

Updated to "jobs", true. Still I think extension client won't work, because jobs use batch client.

Yep, this uses batch client and needs new design which I've introduced in my PetSet PR

Comments from Reviewable

@bryk
Copy link
Contributor

bryk commented Jun 2, 2016

Reviewed 1 of 2 files at r3.
Review status: 8 of 9 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


src/app/backend/resource/common/types.go, line 117 [r2] (raw file):

Previously, floreks (Sebastian Florek) wrote…

Yep, this uses batch client and needs new design which I've introduced in my PetSet PR

Can you @floreks merge and then rebase this PR?

Comments from Reviewable

@floreks
Copy link
Member

floreks commented Jun 2, 2016

We established with @maciaszczykm that I will merge first and then he can use new code in this PR.

@bryk
Copy link
Contributor

bryk commented Jun 2, 2016

Perfect. Fix travis and merge

@maciaszczykm
Copy link
Member Author

Added tests. Fixed JS & Go files formatting (it seems people still forget about it).

@maciaszczykm maciaszczykm merged commit bc3c542 into kubernetes:master Jun 2, 2016
@maciaszczykm maciaszczykm deleted the jobs-backend branch June 2, 2016 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants