Projects tigase _server server-core Issues #705
Prepare description of new DSL format (#705)
Closed
Andrzej Wójcik (Tigase) opened 8 years ago
Due Date
2016-09-22

As of version 7.2.0 it will be possible to use old properties based format and new DSL based format.

We need short description of DSL format in our documentation with some examples of properties file converted into this DSL format.

Andrzej Wójcik (Tigase) commented 8 years ago

I prepared new page in Admin Guide with description of DSL file format.

%wojtek When you will have some time please check if all is explained there and if it is clear. For me it is but I designed this format, so some things may be obvious to me.

wojciech.kapcia@tigase.net commented 8 years ago

A couple of comments. They are all listed in the "as read" order, so if something is not clear but explained later then a short comment or an pointer to the explication would be useful. I've also make some small changes to the document.

In previous Tigase XMPP Server releases configuration was stored in properties based configuration file. From Tigase XMPP Server 7.2.0 release it will be possible

// recommended ??

to use new DSL based configuration file format. This file format was inspired by Groovy language syntax and new core feature of Tigase XMPP Server - Tigase Kernel Framework.

Recommended or enforced?

I've read a comment somewhere, that while possible to use properties file, it will be converted to new DSL file and this file will be used?

Like in properties file it is still possible to use property names starting with -- without any context or any other properties at global scope. Format is the same as in case of setting property but they are defined without scope (in global scope).

Does it require for the property to be enclosed in {} or it can be anywhere in the file? I think that limiting it to single global block would be a good idea (so we would avoid problem with scattering properties all over the place, which you mentioned earlier).

Adding a clarification whether global properties require -- or not to be global property (or we can simply use it without dashes but outside of bean config?) would be a good idea.

(default: if you try to configure bean under name which has default class assigned with it in Tigase framework then this assigned class will be used. In other case you need to pass name of class to use as a bean)

What will happen if there is no default class and no class is specified? Will an exception be thrown?

If you know that bean is defined and you do not want to change it's activity or class then you can just pass properties to configure bean in following way:

How can I know that? By remembering all confing? Will there be something like etc/config-dump.properties (which is IMHO very useful)?

Format values - as a plain string::

Very similar to properties based configuration, in fact values are passed in same format and later are converted to correct type by checking type expected by bean. (Not recommended)

an example? This explication is also confusing (i.e. I'm not sure about what and how should be used, even that it's not recommended)

default () - Configure default data source (using default implementation so class is omitted)

is default a reserved data source?

Enable Stream Management bean

'urn:xmpp:sm:3' () {}

are parenthesis/brackets require, even empty?

    # Add additional port 5224 which is SSL port
    connections () {
        '5224' () {

This was described for values (strings in single-quote, integer as integer) so why now IDs/names seem to use something different? Or to be more precise - why value for port number (which is an integer and is used as such to open new connection) is in single-quotes, which indicates string? For the compatibility with older version (which used strings in port_port_props as keys)?

Another question in this area - how to disable a port? with @active: false@?

Moreover:

  • explicit explenation of multi-level configuration (multi-level beans?); at one point you defined possible value as map, which is simply a K,V list of elements, but the only difference with bean (i.e. ports config as above) is… lack of name of the bean?

  • whitespace? is it important? Can I do:

        '5224' () {
	         socket = ssl, type=accept
	      }
Andrzej Wójcik (Tigase) commented 8 years ago

Thank you for comments. This is a feedback I was expecting to get to improve documentation and to adjust DSL to make it better before we will start using it.

Below are answers to your questions and some comments. Let me know if it clarifies it and if so I will update documentation, if not I will try to answer your other questions.

Wojciech Kapcia wrote:

A couple of comments. They are all listed in the "as read" order, so if something is not clear but explained later then a short comment or an pointer to the explication would be useful. I've also make some small changes to the document.

In previous Tigase XMPP Server releases configuration was stored in properties based configuration file. From Tigase XMPP Server 7.2.0 release it will be possible

// recommended ??

to use new DSL based configuration file format. This file format was inspired by Groovy language syntax and new core feature of Tigase XMPP Server - Tigase Kernel Framework.

Recommended or enforced?

I've read a comment somewhere, that while possible to use properties file, it will be converted to new DSL file and this file will be used?

I other file it is stated that file with configuration in properties file will be converted to DSL, but this will be save to separate file. So it will be possible to still use properties file. I'm not sure if we decide to use DSL as recommended or not so I used possible and marked in comment that we may recommend this new format.

Like in properties file it is still possible to use property names starting with -- without any context or any other properties at global scope. Format is the same as in case of setting property but they are defined without scope (in global scope).

Does it require for the property to be enclosed in {} or it can be anywhere in the file? I think that limiting it to single global block would be a good idea (so we would avoid problem with scattering properties all over the place, which you mentioned earlier).

Adding a clarification whether global properties require -- or not to be global property (or we can simply use it without dashes but outside of bean config?) would be a good idea.

Global properties are just stored outside of any bean config. Can be prefixed with "--" if they need to be set as System property. There is no need enclose properties in {} if they are global properties - in fact this is not allowed (at this point).

At this point there is no "global" block but we can think about it and add it.

This is done this way as there is one additional feature not documented but possible to use in DSL and new Tigase Kernel Framework.

Some configuration properties may be marked with alias in @ConfigField and this allows this fields to be configured using this property from "parent" block (if not exists in parent block, then next parent block is cheked, etc.)

This way is makes it possible to set some settings only once - ie. in this way admins property works. It is a global property by implementation of component has this field annotated with @ConfigField with alias set to admin which make it work.

(default: if you try to configure bean under name which has default class assigned with it in Tigase framework then this assigned class will be used. In other case you need to pass name of class to use as a bean)

What will happen if there is no default class and no class is specified? Will an exception be thrown?

Initialization of this bean is skipped without any exception. This works this way as in OSGi mode it may be possible to configure component for which class implementing a bean is not yet available.

If you know that bean is defined and you do not want to change it's activity or class then you can just pass properties to configure bean in following way:

How can I know that? By remembering all confing? Will there be something like etc/config-dump.properties (which is IMHO very useful)?

Yes, this is something we should add, but it is not supported at this time.

However current state of this is similar to state of XMPP processors configuration in older versions of Tigase - you never know how processors are named and there was no complete list of them. Also there was no mention which are enabled by default.

Format values - as a plain string::

Very similar to properties based configuration, in fact values are passed in same format and later are converted to correct type by checking type expected by bean. (Not recommended)

an example? This explication is also confusing (i.e. I'm not sure about what and how should be used, even that it's not recommended)

We can add examples and more detailed description but it will make this section longer, ie. to pass array of number you could use '5222,5223' like in properties file.

default () - Configure default data source (using default implementation so class is omitted)

is default a reserved data source?

default is name of datasource which will be used by every repository if not configured otherwise. You can think of it as equivalent to --user-repo from older version of Tigase, as this repository URI was used by every repository without specified other URI.

Enable Stream Management bean

'urn:xmpp:sm:3' () {}

are parenthesis/brackets require, even empty?

In current version they are, but I can adjust reader and writer to make them omit requirements for parenthesis if it will be better this way

[...]

This was described for values (strings in single-quote, integer as integer) so why now IDs/names seem to use something different? Or to be more precise - why value for port number (which is an integer and is used as such to open new connection) is in single-quotes, which indicates string? For the compatibility with older version (which used strings in port_port_props as keys)?

No, part before { or before ( is name of bean and it will always be string. However it may contain chars like =,:/ which would need to be escaped in this name, so to make it simple you can wrap whole name in '' and it will be used as single string. However wrapping every name of bean is rather an overkill but it is allowed.

Another question in this area - how to disable a port? with @active: false@?

That is right. Just like any other bean.

Moreover:

  • explicit explenation of multi-level configuration (multi-level beans?); at one point you defined possible value as map, which is simply a K,V list of elements, but the only difference with bean (i.e. ports config as above) is… lack of name of the bean?

Not exactly. If I configure a bean in fact I use a map of values. So

sess-man {
    test = 1,
    other = 'test'
}

is in fact map of key and values to pass to property named sess-man@. This is passed to @bean as it matches name of property at this level of beans tree.

If I would like to pass map to property keys of http component then I could you following entry:

http {
    keys {
        x = 1,
        y = 2
    }
}

and this is equal to:

http {
    keys = {
        x = 1,
        y = 2
    }
}

We can change it to force usage of () {} for beans and = { } for setting values as map if you think this will be better.

  • whitespace? is it important? Can I do:

Yes, you can. I have not explained it as it is not expected format and can make more mess than good.

[...] or does it have to be in separate lines?

I just would it to look like this, it is easier to read.

wojciech.kapcia@tigase.net commented 8 years ago

Andrzej Wójcik wrote:

Recommended or enforced?

I've read a comment somewhere, that while possible to use properties file, it will be converted to new DSL file and this file will be used?

I other file it is stated that file with configuration in properties file will be converted to DSL, but this will be save to separate file. So it will be possible to still use properties file. I'm not sure if we decide to use DSL as recommended or not so I used possible and marked in comment that we may recommend this new format.

OK, but the question is will we have two configuration files on disk and what would be priority? For example someone with init.properties moves to new version, configuration gets converted, someone wants to update configuration, starts to tinker with @init.properties@, this will override everything in DSL always?

If someone would start with fresh install and right with DSL, and then read some old thread mentioning @init.properties@, put some config there, will it wipe changes in DSL or, for example, merge it?

Similar goes for the installer.

Global properties are just stored outside of any bean config. Can be prefixed with "--" if they need to be set as System property.

I would add explanation of this difference to documentation. However - I'm not sure we should have two different "global" variables (i.e. System property and global bean property). It may be allowed for now, but going forward I think we should drop System.properties in favour of new global config while allowing poeple to adjust their components (definitely requires mark in release notes)

There is no need enclose properties in {} if they are global properties - in fact this is not allowed (at this point).

At this point there is no "global" block but we can think about it and add it.

This is done this way as there is one additional feature not documented but possible to use in DSL and new Tigase Kernel Framework.

Some configuration properties may be marked with alias in @ConfigField and this allows this fields to be configured using this property from "parent" block (if not exists in parent block, then next parent block is cheked, etc.)

This way is makes it possible to set some settings only once - ie. in this way admins property works. It is a global property by implementation of component has this field annotated with @ConfigField with alias set to admin which make it work.

OK, now it makes more sense now. Though it should be possible to utilize single "global block" I'm now not sure if its worth additional hassle.

(default: if you try to configure bean under name which has default class assigned with it in Tigase framework then this assigned class will be used. In other case you need to pass name of class to use as a bean)

What will happen if there is no default class and no class is specified? Will an exception be thrown?

Initialization of this bean is skipped without any exception. This works this way as in OSGi mode it may be possible to configure component for which class implementing a bean is not yet available.

A note in the logs that something went wrong at least?

If you know that bean is defined and you do not want to change it's activity or class then you can just pass properties to configure bean in following way:

How can I know that? By remembering all confing? Will there be something like etc/config-dump.properties (which is IMHO very useful)?

Yes, this is something we should add, but it is not supported at this time.

However current state of this is similar to state of XMPP processors configuration in older versions of Tigase - you never know how processors are named and there was no complete list of them. Also there was no mention which are enabled by default.

OK, understood. But in a way - we wanted to avoid such state. I definitely +1 adding this (or improving web-ui where you could see all possible config options and know the state, but this is more for the future, as that would probably require/would be good overwriting config files)

Format values - as a plain string::

Very similar to properties based configuration, in fact values are passed in same format and later are converted to correct type by checking type expected by bean. (Not recommended)

an example? This explication is also confusing (i.e. I'm not sure about what and how should be used, even that it's not recommended)

We can add examples and more detailed description but it will make this section longer, ie. to pass array of number you could use '5222,5223' like in properties file.

I'm definitely in favour of explicit over implicit. It may be obvious to you, especially that you've created the format, but not necessarily for others, especially judging from the questions we face.

Enable Stream Management bean

'urn:xmpp:sm:3' () {}

are parenthesis/brackets require, even empty?

In current version they are, but I can adjust reader and writer to make them omit requirements for parenthesis if it will be better this way

I think it may improve readability, but then again - current way is more explicit we are dealing with a bean config.

[...]

This was described for values (strings in single-quote, integer as integer) so why now IDs/names seem to use something different? Or to be more precise - why value for port number (which is an integer and is used as such to open new connection) is in single-quotes, which indicates string? For the compatibility with older version (which used strings in port_port_props as keys)?

No, part before { or before ( is name of bean and it will always be string. However it may contain chars like =,:/ which would need to be escaped in this name, so to make it simple you can wrap whole name in '' and it will be used as single string. However wrapping every name of bean is rather an overkill but it is allowed.

Makes sense, definitely worth including in the guide.

Another question in this area - how to disable a port? with @active: false@?

That is right. Just like any other bean.

Either a comment or example.

(also what has been mentioned before - a dump of current and/or default configuration would be helpful in this case)

Moreover:

  • explicit explenation of multi-level configuration (multi-level beans?); at one point you defined possible value as map, which is simply a K,V list of elements, but the only difference with bean (i.e. ports config as above) is… lack of name of the bean?

Not exactly. If I configure a bean in fact I use a map of values. So

[...]

is in fact map of key and values to pass to property named sess-man@. This is passed to @bean as it matches name of property at this level of beans tree.

If I would like to pass map to property keys of http component then I could you following entry:

[...]

and this is equal to:

[...]

Can you re-phrase this section? Is it intentional that both blocks are the same?

We can change it to force usage of () {} for beans and = { } for setting values as map if you think this will be better.

IMHO this would improve readability and would make it easier to understand.

  • whitespace? is it important? Can I do:

Yes, you can. I have not explained it as it is not expected format and can make more mess than good.

[...] or does it have to be in separate lines?

I just would it to look like this, it is easier to read.

OK, but this is new format, so we should outline all the rules, because in the real world straying from "expected format" is quite easy, especially when we don't have 100% complete and exact description of the expected format.

Andrzej Wójcik (Tigase) commented 8 years ago

Wojciech Kapcia wrote:

Andrzej Wójcik wrote:

Recommended or enforced?

I've read a comment somewhere, that while possible to use properties file, it will be converted to new DSL file and this file will be used?

I other file it is stated that file with configuration in properties file will be converted to DSL, but this will be save to separate file. So it will be possible to still use properties file. I'm not sure if we decide to use DSL as recommended or not so I used possible and marked in comment that we may recommend this new format.

OK, but the question is will we have two configuration files on disk and what would be priority? For example someone with init.properties moves to new version, configuration gets converted, someone wants to update configuration, starts to tinker with @init.properties@, this will override everything in DSL always?

If someone would start with fresh install and right with DSL, and then read some old thread mentioning @init.properties@, put some config there, will it wipe changes in DSL or, for example, merge it?

Similar goes for the installer.

No, no, no. I check if init.properties is in DSL or in properties format. In in properties then I create new file with converted configuration named @init.properties.new@. To let Tigase use this converted configuration you need to manually replace init.properties file with file with DSL (@init.properties.new).

I now it may be confusing to store DSL in init.properties file, but I decided not to work on this. I wanted to discuss it as we would need to find a new name for DSL config file, establish order/priority of use, etc. I'm open to suggestions.

Global properties are just stored outside of any bean config. Can be prefixed with "--" if they need to be set as System property.

I would add explanation of this difference to documentation. However - I'm not sure we should have two different "global" variables (i.e. System property and global bean property). It may be allowed for now, but going forward I think we should drop System.properties in favour of new global config while allowing poeple to adjust their components (definitely requires mark in release notes)

I agree, that this would be best in future. I used this in same way as it was used in init.properties to make it compatible.

There is no need enclose properties in {} if they are global properties - in fact this is not allowed (at this point).

At this point there is no "global" block but we can think about it and add it.

This is done this way as there is one additional feature not documented but possible to use in DSL and new Tigase Kernel Framework.

Some configuration properties may be marked with alias in @ConfigField and this allows this fields to be configured using this property from "parent" block (if not exists in parent block, then next parent block is cheked, etc.)

This way is makes it possible to set some settings only once - ie. in this way admins property works. It is a global property by implementation of component has this field annotated with @ConfigField with alias set to admin which make it work.

OK, now it makes more sense now. Though it should be possible to utilize single "global block" I'm now not sure if its worth additional hassle.

It could be good to have this, but we would need to create some specific block with special name and adjust a code to this workaround.

(default: if you try to configure bean under name which has default class assigned with it in Tigase framework then this assigned class will be used. In other case you need to pass name of class to use as a bean)

What will happen if there is no default class and no class is specified? Will an exception be thrown?

Initialization of this bean is skipped without any exception. This works this way as in OSGi mode it may be possible to configure component for which class implementing a bean is not yet available.

A note in the logs that something went wrong at least?

Yes, this is something we can do. I still need to test kernel with OSGi and adjust it if needed.

If you know that bean is defined and you do not want to change it's activity or class then you can just pass properties to configure bean in following way:

How can I know that? By remembering all confing? Will there be something like etc/config-dump.properties (which is IMHO very useful)?

Yes, this is something we should add, but it is not supported at this time.

However current state of this is similar to state of XMPP processors configuration in older versions of Tigase - you never know how processors are named and there was no complete list of them. Also there was no mention which are enabled by default.

OK, understood. But in a way - we wanted to avoid such state. I definitely +1 adding this (or improving web-ui where you could see all possible config options and know the state, but this is more for the future, as that would probably require/would be good overwriting config files)

I think we could do this and is should not be very difficult. For that we can create separate task.

Format values - as a plain string::

Very similar to properties based configuration, in fact values are passed in same format and later are converted to correct type by checking type expected by bean. (Not recommended)

an example? This explication is also confusing (i.e. I'm not sure about what and how should be used, even that it's not recommended)

We can add examples and more detailed description but it will make this section longer, ie. to pass array of number you could use '5222,5223' like in properties file.

I'm definitely in favour of explicit over implicit. It may be obvious to you, especially that you've created the format, but not necessarily for others, especially judging from the questions we face.

I agree. So we have 2 solution:

  • add explanation

  • remove any mention of this format

Enable Stream Management bean

'urn:xmpp:sm:3' () {}

are parenthesis/brackets require, even empty?

In current version they are, but I can adjust reader and writer to make them omit requirements for parenthesis if it will be better this way

I think it may improve readability, but then again - current way is more explicit we are dealing with a bean config.

Right, I think we could remove this requirement to improve readability.

[...]

This was described for values (strings in single-quote, integer as integer) so why now IDs/names seem to use something different? Or to be more precise - why value for port number (which is an integer and is used as such to open new connection) is in single-quotes, which indicates string? For the compatibility with older version (which used strings in port_port_props as keys)?

No, part before { or before ( is name of bean and it will always be string. However it may contain chars like =,:/ which would need to be escaped in this name, so to make it simple you can wrap whole name in '' and it will be used as single string. However wrapping every name of bean is rather an overkill but it is allowed.

Makes sense, definitely worth including in the guide.

Ok, next thing which should be added to guide.

Another question in this area - how to disable a port? with @active: false@?

That is right. Just like any other bean.

Either a comment or example.

Will add it when work on rest of mentioned issues.

(also what has been mentioned before - a dump of current and/or default configuration would be helpful in this case)

Moreover:

  • explicit explenation of multi-level configuration (multi-level beans?); at one point you defined possible value as map, which is simply a K,V list of elements, but the only difference with bean (i.e. ports config as above) is… lack of name of the bean?

Not exactly. If I configure a bean in fact I use a map of values. So

[...]

is in fact map of key and values to pass to property named sess-man@. This is passed to @bean as it matches name of property at this level of beans tree.

If I would like to pass map to property keys of http component then I could you following entry:

[...]

and this is equal to:

[...]

Can you re-phrase this section? Is it intentional that both blocks are the same?

We can change it to force usage of () {} for beans and = { } for setting values as map if you think this will be better.

IMHO this would improve readability and would make it easier to understand.

Will change DSL format reader and write to use it and adjust guide.

  • whitespace? is it important? Can I do:

Yes, you can. I have not explained it as it is not expected format and can make more mess than good.

[...] or does it have to be in separate lines?

I just would it to look like this, it is easier to read.

OK, but this is new format, so we should outline all the rules, because in the real world straying from "expected format" is quite easy, especially when we don't have 100% complete and exact description of the expected format.

OK, will add comments to guide to explain possible formats and suggested one to use.

Anything else?

wojciech.kapcia@tigase.net commented 8 years ago

Andrzej Wójcik wrote:

Wojciech Kapcia wrote:

Andrzej Wójcik wrote:

Recommended or enforced?

I've read a comment somewhere, that while possible to use properties file, it will be converted to new DSL file and this file will be used?

I other file it is stated that file with configuration in properties file will be converted to DSL, but this will be save to separate file. So it will be possible to still use properties file. I'm not sure if we decide to use DSL as recommended or not so I used possible and marked in comment that we may recommend this new format.

OK, but the question is will we have two configuration files on disk and what would be priority? For example someone with init.properties moves to new version, configuration gets converted, someone wants to update configuration, starts to tinker with @init.properties@, this will override everything in DSL always?

If someone would start with fresh install and right with DSL, and then read some old thread mentioning @init.properties@, put some config there, will it wipe changes in DSL or, for example, merge it?

Similar goes for the installer.

No, no, no. I check if init.properties is in DSL or in properties format. In in properties then I create new file with converted configuration named @init.properties.new@. To let Tigase use this converted configuration you need to manually replace init.properties file with file with DSL (@init.properties.new).

I now it may be confusing to store DSL in init.properties file, but I decided not to work on this. I wanted to discuss it as we would need to find a new name for DSL config file, establish order/priority of use, etc. I'm open to suggestions.

Considering that we by default utilize etc/init.properties (referenced from scripts/tigase.sh and sysinit scripts) it may be better to make a backup of the init.properties in propeperties format (i.e. @etc/init.properties.back/save@) and put the new config in the @etc/init.properties@. Of course appropriate notification would be included in tigase-console.log (in a noticeable manner).

From your description above I would assume one would have to:

  • start tigase

  • tigase detects old format, creates etc/init.properties.new

  • user replaces etc/init.properties with etc/init.properties.new

  • user have to restart tigase

  • is this correct?

There is no need enclose properties in {} if they are global properties - in fact this is not allowed (at this point).

At this point there is no "global" block but we can think about it and add it.

This is done this way as there is one additional feature not documented but possible to use in DSL and new Tigase Kernel Framework.

Some configuration properties may be marked with alias in @ConfigField and this allows this fields to be configured using this property from "parent" block (if not exists in parent block, then next parent block is cheked, etc.)

This way is makes it possible to set some settings only once - ie. in this way admins property works. It is a global property by implementation of component has this field annotated with @ConfigField with alias set to admin which make it work.

OK, now it makes more sense now. Though it should be possible to utilize single "global block" I'm now not sure if its worth additional hassle.

It could be good to have this, but we would need to create some specific block with special name and adjust a code to this workaround.

Wouldn't a block without a name work in this case? We could create some "restricted" name (e.g.: global)?

However after pondering it some more I think that leaving it as-is without additional block as current structure is more logical.

(default: if you try to configure bean under name which has default class assigned with it in Tigase framework then this assigned class will be used. In other case you need to pass name of class to use as a bean)

What will happen if there is no default class and no class is specified? Will an exception be thrown?

Initialization of this bean is skipped without any exception. This works this way as in OSGi mode it may be possible to configure component for which class implementing a bean is not yet available.

A note in the logs that something went wrong at least?

Yes, this is something we can do. I still need to test kernel with OSGi and adjust it if needed.

I think this is more of must-have -- it was a bit troubling a while back when some components weren't loaded without any visible mark and hint as to why...

If you know that bean is defined and you do not want to change it's activity or class then you can just pass properties to configure bean in following way:

How can I know that? By remembering all confing? Will there be something like etc/config-dump.properties (which is IMHO very useful)?

Yes, this is something we should add, but it is not supported at this time.

However current state of this is similar to state of XMPP processors configuration in older versions of Tigase - you never know how processors are named and there was no complete list of them. Also there was no mention which are enabled by default.

OK, understood. But in a way - we wanted to avoid such state. I definitely +1 adding this (or improving web-ui where you could see all possible config options and know the state, but this is more for the future, as that would probably require/would be good overwriting config files)

I think we could do this and is should not be very difficult. For that we can create separate task.

#4547

Ideally in the same format.

Format values - as a plain string::

Very similar to properties based configuration, in fact values are passed in same format and later are converted to correct type by checking type expected by bean. (Not recommended)

an example? This explication is also confusing (i.e. I'm not sure about what and how should be used, even that it's not recommended)

We can add examples and more detailed description but it will make this section longer, ie. to pass array of number you could use '5222,5223' like in properties file.

I'm definitely in favour of explicit over implicit. It may be obvious to you, especially that you've created the format, but not necessarily for others, especially judging from the questions we face.

I agree. So we have 2 solution:

  • add explanation
  • remove any mention of this format

+1 for the former. Removing any mention while still making it possibility would inevitably lead to questions "why is that used?"

OK, will add comments to guide to explain possible formats and suggested one to use.

Anything else?

For now everything seems to be ironed out. There is a possibility that something may come up while working with 7.2.0 on daily basis (or ideas for some improvements?) but for now looks good.

Andrzej Wójcik (Tigase) commented 8 years ago

I applied most of suggested changes. I hope I did miss any.

However I have one issue with requirement of usage = for assigning map as a value and force usage of beanName() {} for assigning bean properties. This is very difficult as during conversion from properties format, I may find following entries:

sess-man/plugins-conf/test1/key1=val1
sess-man/plugins-conf/test1/key2=val2
sess-man/plugins-conf/test1/key3=val3

which is written as a map

sess-man {
    test1 {
        key1 = 'val1'
        key2 = 'val1'
        key3 = 'val1'
    }
}

and after required changes would be written as:

sess-man {
    test1 = {
        key1 = 'val1'
        key2 = 'val1'
        key3 = 'val1'
    }
}

, but key1@, @key2 and key3 are properties of a bean so it should be bean definition like in this example:

sess-man {
    test1() {
        key1 = 'val1'
        key2 = 'val1'
        key3 = 'val1'
    }
}

So I think we should not introduce requirement to use = to assign property value and not to assign values of a bean as this will cause issues with migration from old configuration files.

wojciech.kapcia@tigase.net commented 8 years ago

Andrzej Wójcik wrote:

I applied most of suggested changes. I hope I did miss any.

I've went over updated guide and it looks better and it's more clear. Thank you.

However I have one issue with requirement of usage = for assigning map as a value and force usage of beanName() {} for assigning bean properties. This is very difficult as during conversion from properties format, I may find following entries:

[...]

which is written as a map

[...]

and after required changes would be written as:

[...]

, but key1@, @key2 and key3 are properties of a bean so it should be bean definition like in this example:

[...]

So I think we should not introduce requirement to use = to assign property value and not to assign values of a bean as this will cause issues with migration from old configuration files.

Wouldn't it be possible to handle this exceptional cases separately? Right now I can think of ports config, plugins configuration. How do you determine during loading of the configuration whether this is a bean configuration or a map? Wouldn't it be possible to apply same principles during conversion?

Andrzej Wójcik (Tigase) commented 8 years ago

There is very small difference between bean configuration and a map. In fact bean configuration is extended map.

During initialization I look for bean definitions to register new beans or update bean definition. Later on I use bean name to look for map of key-value for configuration properties of a bean. If I will find map instead of bean definition, it still will be used as it is a map and name matches.

wojciech.kapcia@tigase.net commented 8 years ago

Thank you for clarification. I consider this resolved now.

issue 1 of 1
Type
Task
Priority
Normal
Assignee
RedmineID
4481
Version
tigase-server-8.0.0
Spent time
67h 30m
Issue Votes (0)
Watchers (0)
Reference
tigase/_server/server-core#705
Please wait...
Page is in error, reload to recover