tosca message
[Date Prev]
| [Thread Prev]
| [Thread Next]
| [Date Next]
--
[Date Index]
| [Thread Index]
| [List Home]
Subject: RE: [tosca] Groups - 2015-04-19 tosca_elk_DRAFT_CSAR.zip uploaded
- From: Sahdev P Zala <spzala@us.ibm.com>
- To: Chris Lauwers <lauwers@ubicity.com>
- Date: Mon, 20 Apr 2015 18:48:53 -0400
Hi Chris,
Appreciate your thorough review. Very
helpful.
I am still little confused about MyConnectTo
so other than that I have replied your comments below in-line. Most of
them are straightforward except few where I have question/comment. Please
take a look.
Since how we should deal with custom
relationship is under discussion, I think it would be a quicker and better
to discuss it during one of our group calls.
Thanks!
Regards,
Sahdev Zala
From:
Chris Lauwers <lauwers@ubicity.com>
To:
Sahdev P Zala/Durham/IBM@IBMUS,
"tosca@lists.oasis-open.org" <tosca@lists.oasis-open.org>
Date:
04/20/2015 04:26 PM
Subject:
RE: [tosca]
Groups - 2015-04-19 tosca_elk_DRAFT_CSAR.zip uploaded
Thanks Sahdev, but I don’t
believe this change fixes the problem, it only moves it somewhere else.
Yes, we now have a new MyConnectsTo relationship type, but MyConnectsTo
implicitly creates a new Interface Type (based on tosca.interfaces.relationship.Configure)
that adds a new input parameter. If we’re not allowed to dynamically “augment”
types in entity definitions, then clearly augmenting Interface Types shouldn’t
be allowed either. The correct solution here is to explicitly introduce
a new Interface Type (e.g. MyConfigure) that derives from Configure and
then have MyConnectsTo use the MyConfigure interface.
In addition, your change
doesn’t address the corresponding issue in requirement assignments. For
example, the logstash node template uses an “in-line” relationship assignment
syntax to assign an IP address value to the IP address input defined in
the MyConnectsTo relationship type. According to the spec, this is not
allowed. Instead, a new relationship template needs to be defined explicitly.
(Note that my argument here
is solely based on what’s in the spec. I want to re-iterate my personal
opinion that the spec is too restrictive and that what you were trying
to do in the first version of this file is perfectly acceptable and should
be allowed).
Aside from the requirements-related
issues, here are some other comments (in no particular order)
1. The
tosca_base_type_definition.yaml file is not a valid Tosca file (i.e. it
doesn’t include any valid Tosca keynames) and as such should not be used
in an “import” statement
(sahdev) Sorry but I didn't exactly get it. To me, this file represents
normative types and related definition as defined in the spec. I am specially
confused when you said " it doesn’t include any valid Tosca keynames".
I am fine to take it out of CSAR per my answer to your next comment.
2. More
importantly, in my opinion it should be the responsibility of the Tosca
runtime to import all the normative type definitions, not the responsibility
of the application developer.
(sahdev) OK, that's fine. Actually, that's how we do it in heat-translator.
I put in import thinking that if someone want to try CSAR as such in an
environment where normative type aren't handled at runtime then also CSAR
will work. I am good removing related import statement if all agree and
in that case I think it's a good idea to update README about assumption
we are making here.
3. The
PayPalPizzaStore node type defines a “database_connection” capability
with capability type tosca.capabilities.DatabaseEndpoint. There is no such
type in the normative type definitions. The correct type is tosca.capabilities.Endpoint.Database
(sahdev) Yes, my bad.
It was there but I missed the latest changes. I will fix it.
4. A
number of node templates include a “get_attribute[<server>, ip_address]”
where <server> is an instance of Compute. However, Compute no longer
has IP address attributes. For this use case, I believe we need to use
“private_address” instead.
(sahdev) Yes, agree. I
missed the latest in spec here as well.
5. In
the MyConnectsTo relationship type definition, you reference the Configure
interface by its type name (tosca.interfaces.relationship.Configure) rather
than by its symbolic name (which is “Configure”).
(sahdev) Sorry here too
and for following two comments, but I think it's up to template author
whether to use the full name or short?
6. You
have a similar problem in the PayPalPizzaStore node type: you refer the
“Standard” interface by its type name (tosca.interfaces.node.lifecycle.Standard)
rather than by its symbolic name.
7. This
problem then carries over into all the node templates as well (about a
dozen instances). Whenever you reference an interface, you refer to that
interface by its type name rather than by its symbolic name.
8. Your
“list” syntax is incorrect for requirements definitions in logstash.yaml.
There is a missing “dash” before the “search_endpoint” requirement
name. The correct syntax is as follows:
(sahdev) Ah, it was correct in the previous draft but I missed
"dash" in the current draft while making changes. I will fix
it.
node_types:
tosca.nodes.SoftwareComponent.Logstash:
derived_from:
tosca.nodes.SoftwareComponent
requirements:
- search_endpoint:
capability: tosca.capabilities.Endpoint
node: tosca.nodes.SoftwareComponent.Elasticsearch
relationship: tosca.relationships.MyConnectsTo
9. Whenever
you declare inputs to an Operation in an Interface, you use the “input”
keyname rather than the “inputs” keyname. I don’t believe “input”
is a valid keyname anywhere in the Tosca spec.
(sahdev) Yup, agree. I
have used it correctly certain places and missed it at few places.
I will fix it.
10. The “mongo_db”
node template fails to set the following required properties: “user”,
“name”, “port”, and “password”.
(sahdev) I have a point
to make here, the default installation of mongoDB doesn't require user,
password, port or db name. The scripts we have doesn't require any of these.
When we created this use case, I found from people who have worked with
mongo db that the default implementation is a common usage and provides
a simple configuration. Using mongoDB as an example, I feel that the properties
for Database and DBMS should be optional instead of required. Something
to discuss in our YAML call.
11. The “paypal_pizzastore”
node template fails to set the required property “context_root”
(sahdev) Hmm... The spec
has “context_root” as optional an optional property for WebApplication
and I think the template is not explicitly mentioned it anywhere. Please
help me understand.
12. The “mongo_dbms”
node template fails to set the required properties “port” and “root_password”
(sahdev) (Same as my reply
for #10.)
13. In the “interfaces”
section of the “paypal_pizzastore” node template, the “mongodb_ip”
and “github_url” inputs are assigned only for the “configure” operation,
but in the PayPalPizzaStore node type these inputs are declared for all
operations in the “Standard” interface. The correct assignment is as
follows:
(sahdev)
I see. Since these inputs are only required for 'configure', may be better
to update the custom type definition for paypay_pizzastore though I agree
what you recommended correct the grammar.
node_templates:
paypal_pizzastore:
type:
tosca.nodes.WebApplication.PayPalPizzaStore
interfaces:
Standard:
inputs:
github_url: { get_property: [ SELF, github_url ] }
mongodb_ip: { get_attribute: [mongo_server, private_address]
}
configure:
implementation: Scripts/nodejs/configure.sh
start: Scripts/nodejs/start.sh
Let me know if you need more
detail on any of these,
Best regards,
Chris
From: tosca@lists.oasis-open.org
[mailto:tosca@lists.oasis-open.org]
On Behalf Of Sahdev Zala
Sent: Sunday, April 19, 2015 8:17 PM
To: tosca@lists.oasis-open.org
Subject: [tosca] Groups - 2015-04-19 tosca_elk_DRAFT_CSAR.zip uploaded
Submitter's message
Hello, in this draft, I have tried modifying the requirements section per
comments from Chris Lauwers. For logstash types I have created a custom
relationship for now as the topic is still under discussion. Thanks!
-- Sahdev Zala
[Date Prev]
| [Thread Prev]
| [Thread Next]
| [Date Next]
--
[Date Index]
| [Thread Index]
| [List Home]