introduce api/mixer/adapter/model/v1beta #359

guptasu · 2018-02-06T00:26:27Z

introduce api/mixer/adapter/model/v1beta
As per the plan https://docs.google.com/document/d/1C_NMYj9ZhdUWwZMWUoX_SLU8VoiV8U8V6yUJVYcdSS8/edit?ts=5a7220f9#

geeknoid · 2018-02-06T00:31:17Z

mixer/adapter/model/v1beta/BUILD

@@ -0,0 +1,26 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")


Nuke this file, no bazel no more!

geeknoid · 2018-02-06T00:32:03Z

mixer/adapter/model/v1beta/extensions.proto

+package istio.mixer.adapter.model.v1beta;
+import "google/protobuf/descriptor.proto";
+
+// Specifies the variety of the the Template.


// The available varieties of templates, controlling the semantics of what an adapter does with each instance.

geeknoid · 2018-02-06T00:32:31Z

mixer/adapter/model/v1beta/extensions.proto

+
+// Specifies the variety of the the Template.
+enum TemplateVariety {
+    // TEMPLATE_VARIETY_CHECK makes the template applicable for Mixer's check calls.


No need to repeat the enum's name in the description text, it doesn't look good in the generated docs.

geeknoid · 2018-02-06T00:34:16Z

mixer/adapter/model/v1beta/value_type.proto

+
+syntax = "proto3";
+
+package istio.mixer.adapter.model.v1beta;


One of the protos in this package needs to have a package-level comment. This will be used at the top of the page on istio.io.

Additionally, one of the protos needs to have a comment that contains $location, $title, and $overview which are used to drive code gen. See other protos in istio/api for examples.

Will fix it. Do you think we should fail the BUILD if the package is missing these necessary stuff. I remember you already have warnings for that stuff in your tool.

guptasu

Thanks @geeknoid for the feedback. PTAL at the update.

guptasu · 2018-02-06T00:45:42Z

mixer/adapter/model/v1beta/extensions.proto

+package istio.mixer.adapter.model.v1beta;
+import "google/protobuf/descriptor.proto";
+
+// Specifies the variety of the the Template.


guptasu · 2018-02-06T00:46:10Z

mixer/adapter/model/v1beta/extensions.proto

+
+// Specifies the variety of the the Template.
+enum TemplateVariety {
+    // TEMPLATE_VARIETY_CHECK makes the template applicable for Mixer's check calls.


guptasu · 2018-02-06T01:37:53Z

mixer/adapter/model/v1beta/BUILD

@@ -0,0 +1,26 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")


guptasu · 2018-02-06T01:39:25Z

mixer/adapter/model/v1beta/value_type.proto

+
+syntax = "proto3";
+
+package istio.mixer.adapter.model.v1beta;


Will fix it. Do you think we should fail the BUILD if the package is missing these necessary stuff. I remember you already have warnings for that stuff in your tool.

guptasu · 2018-02-06T01:53:50Z

mixer/adapter/model/v1beta/value_type.proto

+
+syntax = "proto3";
+
+package istio.mixer.adapter.model.v1beta;


wora · 2018-02-06T04:17:23Z

mixer/adapter/model/v1beta/standard_types.proto

+
+// Value is used inside templates for fields that have dynamic types. The actual datatype
+// of the field depends on the datatype of the expression used in the operator configuration.
+message Value {}


If you want to define standard types, you should define it under istio.type package and promise never making any breaking change. Don't call it standard if you cannot commit to it.

These are meant to be used only for templates, so putting inside istio.type might be too much. However, I agree standard is not necessary, I can fix it, will rename it to type.proto. WDYT ?

wora · 2018-02-06T04:17:56Z

mixer/adapter/model/v1beta/value_type.proto

+// are used to describe the type of Attributes at run time, describe the type of
+// the result of evaluating an expression, and to describe the runtime type of
+// fields of other descriptors.
+enum ValueType {


Why don't we use attribute type?

@wora which attribute type. AttributeManifest is actually using ValueType. Am I missing something ?

geeknoid · 2018-02-06T05:46:07Z

mixer/adapter/model/v1beta/standard_types.proto

+
+syntax = "proto3";
+
+// This proto describes the types that can be used inside Mixer templates. These message types are used to specify


Only one package-level comment per package, not one per file.

Thanks

Oops. This was already there, forgot to remove it.

guptasu

PTAL

* transcoding: Remove MethodInfo * fix test lib path * fix test

Automatic merge from submit-queue. [DO NOT MERGE] Auto PR to update dependencies of mixerclient This PR will be merged automatically once checks are successful. ```release-note none ```

guptasu added 2 commits February 5, 2018 16:10

Move template and valuetype to mixer/adapter/model/v1beta

91519b0

Plump the mixer/adapter/model/v1beta into generation script

f46f71f

guptasu requested review from douglas-reid and geeknoid February 6, 2018 00:26

guptasu requested a review from rshriram as a code owner February 6, 2018 00:26

googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 6, 2018

geeknoid reviewed Feb 6, 2018

View reviewed changes

Address feedback comments.

89f47d4

guptasu commented Feb 6, 2018

View reviewed changes

wora reviewed Feb 6, 2018

View reviewed changes

geeknoid reviewed Feb 6, 2018

View reviewed changes

Address feedback.

1fc3f18

guptasu commented Feb 6, 2018

View reviewed changes

geeknoid approved these changes Feb 6, 2018

View reviewed changes

guptasu added 2 commits February 5, 2018 22:45

fixed MakeFile

7574ea2

rerun make generate

6a276a5

guptasu merged commit 12edb32 into istio:master Feb 6, 2018

incfly pushed a commit to incfly/api that referenced this pull request Jun 13, 2018

transcoding: Remove MethodInfo (istio#359)

7d01533

* transcoding: Remove MethodInfo * fix test lib path * fix test

nacx added a commit to nacx/api that referenced this pull request Feb 23, 2022

Fix the remaining direct mode annotations (istio#359)

c5263f1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

introduce api/mixer/adapter/model/v1beta #359

introduce api/mixer/adapter/model/v1beta #359

guptasu commented Feb 6, 2018

geeknoid Feb 6, 2018

guptasu Feb 6, 2018

geeknoid Feb 6, 2018

guptasu Feb 6, 2018

geeknoid Feb 6, 2018

guptasu Feb 6, 2018

geeknoid Feb 6, 2018

guptasu Feb 6, 2018

guptasu Feb 6, 2018

guptasu left a comment

guptasu Feb 6, 2018

guptasu Feb 6, 2018

guptasu Feb 6, 2018

guptasu Feb 6, 2018

guptasu Feb 6, 2018

wora Feb 6, 2018

guptasu Feb 6, 2018

wora Feb 6, 2018

guptasu Feb 6, 2018

geeknoid Feb 6, 2018

guptasu Feb 6, 2018

guptasu left a comment

		@@ -0,0 +1,26 @@
		load("@io_bazel_rules_go//go:def.bzl", "go_library")


		syntax = "proto3";

		// This proto describes the types that can be used inside Mixer templates. These message types are used to specify

introduce api/mixer/adapter/model/v1beta #359

introduce api/mixer/adapter/model/v1beta #359

Conversation

guptasu commented Feb 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guptasu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guptasu left a comment

Choose a reason for hiding this comment