Skip to content

Apa Template bootstrap codegen.#1960

Merged
geeknoid merged 4 commits intoistio:masterfrom
guptasu:apa5
Dec 5, 2017
Merged

Apa Template bootstrap codegen.#1960
geeknoid merged 4 commits intoistio:masterfrom
guptasu:apa5

Conversation

@guptasu
Copy link
Copy Markdown
Contributor

@guptasu guptasu commented Dec 1, 2017

What this PR does / why we need it:
It does the generation of instances that are dispatched to APA adapters
and also generates new attributes from the response of the adapter.

This PR finishes the APA support in Mixer.

Next:

  • Change the Kubernetes adapter to use the new model.
  • Finally cleanup the old model.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

none

@istio-testing
Copy link
Copy Markdown
Collaborator

@guptasu: Adding do-not-merge/release-note-label-needed because the release note process has not been followed.

Details

One of the following labels is required "release-note", "release-note-action-required", or "release-note-none".
Please see: https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md#write-release-notes-if-needed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 5, 2017

Codecov Report

Merging #1960 into master will decrease coverage by 2.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #1960     +/-   ##
=========================================
- Coverage   81.81%   79.72%   -2.1%     
=========================================
  Files         190       70    -120     
  Lines       15675     6731   -8944     
=========================================
- Hits        12825     5366   -7459     
+ Misses       2414     1088   -1326     
+ Partials      436      277    -159
Flag Coverage Δ
#broker 47.27% <ø> (ø) ⬆️
#mixer ?
#pilot 80.7% <ø> (+0.12%) ⬆️
#security 92.03% <ø> (ø) ⬆️
Impacted Files Coverage Δ
broker/pkg/model/config/mock_store.go 18.57% <ø> (ø) ⬆️
mixer/pkg/config/descriptor/validator.go
mixer/pkg/mockapi/handler.go
mixer/adapter/statsd/statsd.go
mixer/pkg/adapterManager/logger.go
mixer/adapter/stackdriver/metric/distribution.go
mixer/adapter/svcctrl/testhelper.go
mixer/pkg/adapter/configError.go
mixer/pkg/pool/buffer.go
mixer/adapter/kubernetes/cache.go
... and 118 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 620a7c7...f00d1b2. Read the comment docs.

@guptasu
Copy link
Copy Markdown
Contributor Author

guptasu commented Dec 5, 2017

@geeknoid @douglas-reid @mandarjog @ozevren friendly ping.

Copy link
Copy Markdown
Contributor

@geeknoid geeknoid left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@istio-merge-robot
Copy link
Copy Markdown

/test all [submit-queue is verifying that this PR is safe to merge]

@guptasu
Copy link
Copy Markdown
Contributor Author

guptasu commented Dec 5, 2017

/retest

@guptasu
Copy link
Copy Markdown
Contributor Author

guptasu commented Dec 5, 2017

/test all

@elevran
Copy link
Copy Markdown
Contributor

elevran commented Dec 5, 2017

bazel vs go toolchain issue. This seems to happen for quite a few PRs that introduce new functionality that uses auto-generated files.

+ go test -coverprofile=/tmp/coverage/istio.io-istio-mixer-template-sample-quota.txt istio.io/istio/mixer/template/sample/quota
# istio.io/istio/mixer/template/sample
mixer/template/sample/template.gen_test.go:39:2: no Go files in /go/src/istio.io/istio/mixer/template/sample/apa
FAIL	istio.io/istio/mixer/template/sample [setup failed]

Generated files are not present in package directory which is then imported. Import of package with no Go files is an error.

@guptasu
Copy link
Copy Markdown
Contributor Author

guptasu commented Dec 5, 2017 via email

It does the generation of instances that are dispatched to APA adapters
and also generates new attributes from the response of the adapter.

This PR finishes the APA support in Mixer.

Next:
- Change the Kubernetes adapter to use the new model.
- Finally cleanup the old model.
@istio-merge-robot
Copy link
Copy Markdown

/lgtm cancel //PR changed after LGTM, removing LGTM. @geeknoid @guptasu

@geeknoid
Copy link
Copy Markdown
Contributor

geeknoid commented Dec 5, 2017

/lgtm
/approve

@istio-merge-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geeknoid

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@istio-merge-robot
Copy link
Copy Markdown

/test all [submit-queue is verifying that this PR is safe to merge]

@vadimeisenbergibm
Copy link
Copy Markdown
Contributor

@guptasu Note that go test is used by https://circleci.com/gh/istio/istio, so it is required. circle-ci is one of the Continuous Integration checks of the PRs.

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.

7 participants