synapse-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruwan Linton <ruwan.lin...@gmail.com>
Subject Re: [VOTE] Release Synapse-2.0.0 (Take2)
Date Thu, 16 Dec 2010 16:25:47 GMT
Cool, I am planning to start the build shortly, it might extend to tomorrow
to get the artifacts published for the vote.

Thanks,
Ruwan

On Thu, Dec 16, 2010 at 6:32 PM, Hubert, Eric <Eric.Hubert@foxmobile.com>wrote:

>  Hi Ruwan,
>
>
>
> there is not much to worry about. I have been able to rather easily fix all
> of those issues. I have been only a bit concerned about users which are not
> familiar with the Synapse code at all.
>
>
>
> As you said, the errors regarding SynapseArtifact.getDescription() came
> from custom mediator implementations directly implementing the mediator
> interface without using the abstract base implementation. I do not know how
> often people might have done this. For some of our mediators there was no
> need to extend from the base implementation as it offered nothing meaningful
> for those cases. I guess with Synapse 2.0 users are now strongly encouraged
> to extend the base implementation. Otherwise they have to implement some
> aspects they do not like to care about at all.
>
>
>
> Thanks for the explanation regarding the properties usage. Now it’s purpose
> is clear. Just from looking at the API and Unit-Tests (which is mostly a
> good start to understand something) I wasn’t able to get it.
>
>
>
> Thanks,
>
>    Eric
>
>
>
>
>   ------------------------------
>
> *From:* Ruwan Linton [mailto:ruwan.linton@gmail.com]
> *Sent:* Thursday, December 16, 2010 12:42 PM
>
> *To:* dev@synapse.apache.org
> *Subject:* Re: [VOTE] Release Synapse-2.0.0 (Take2)
>
>
>
> Hi Eric,
>
>
>
> Going through your documentation and trying to document/fix them... here
> are my comments/explanations. :-)
>
> On Fri, Dec 10, 2010 at 1:11 PM, Hubert, Eric <Eric.Hubert@foxmobile.com>
> wrote:
>
> I found quite a bit of time to have a more detailed look at the
> incompatible changes and start with an upgrade process - comments inline.
>   ------------------------------
>
> *From:* Hubert, Eric [mailto:Eric.Hubert@foxmobile.com]
> *Sent:* Thursday, December 09, 2010 12:40 PM
>
>
> *To:* dev@synapse.apache.org
> *Subject:* RE: [VOTE] Release Synapse-2.0.0 (Take2)
>
>
>
> Here is a list of incompatibilities immediately found. Some are only from
> unit tests – not all will be considered as public API. To solve some tasks
> access was needed for different reasons…
>
> Somehow sorted by importance:
>
>
>
> The type XYZ must implement the inherited abstract method
> AbstractMediatorFactory.createSpecificMediator(OMElement, Properties)
>
>
>
> This has been documented on the upgrading guide
>
>
>
>   The type XYZ must implement the inherited abstract method
> SynapseArtifact.getDescription()
>
> The type XYZ must implement the inherited abstract method
> SynapseArtifact.setDescription(String)
>
>
>
> From where did you get these errors, you cannot get these errors if you are
> extending from the abstract classes, since it contains these
> implementations, and the description is automatically available for the
> programmer through the abstract class. This is one of the main ideas of the
> above API refactoring too.
>
>
>
>   Cannot override the final method from AbstractMediatorSerializer
>
>
>
> This is caused by the serializeSpecificMediator introduction just as in the
> factory, and you should use that instead of the final method. And yes this
> has been documented too.
>
>
>
>   The field AbstractMediatorFactory.log is not visible
>
>
>
> This seems to be mistakenly changed by Hiranya to package private from
> protected, I have reverted it to be protected so that you can use it now,
> sorry for the inconveniences.
>
>
>
>   The method createMediator(OMElement, Properties) in the type
> AbstractMediatorFactory is not applicable for the arguments (OMElement)
>
> The method createMediator(OMElement, Properties) in the type
> MediatorFactory is not applicable for the arguments (OMElement)
>
> The method createMediator(OMElement) of type XYZ must override or implement
> a supertype method
>
>
>
> The above 3 cases has already been taken into account in the documentation
> of AbstractMediatorFactory/Serializer change.
>
>
>
>
>
> All of the above is what I assume could/should be considered public API and
> all of the changes required to make custom mediator code compile again are
> trivial and quickly done. I did this for a couple of mediators.
>
> I have still one question: What has been the reason to add the Properties
> to the createSpecificMediator method in the AbstractMediatorFactory? I tried
> to answer this question myself and looked for usage of the properties in the
> source. I only checked a couple of implementation, but could nowhere find a
> use. Everywhere this parameter was ignored in the Factory implementations
> and callers passed in an empty Properties-Map.
>
>
>
> This is to resolve the coupling between the server startup with Axis2
> integration and Synapse configuration building. Those are two concerns and
> earlier the ServerConfigurationInformation class was a singleton and the
> rest of the synapse core code has mis used this singleton behavior to get
> the access to information like SYNAPSE_HOME and RESOLVE_ROOT. These
> information really are a set of arguments that needs to be passed into the
> configuration building framework, but I thought ahead a little bit about
> what if we have more parameters in the future, and introduced a generic
> properties map.
>
>
>
> See the Axis2SynapseController#createSynapseConfiguration method to see how
> we pass in these parameters to the configuration building framework removing
> the coupling.
>
>
>
>
>
>
>
> The method getMediator(OMElement, Properties) in the type
> MediatorFactoryFinder is not applicable for the arguments (OMElement)
>
> The method getMediator(OMElement, Properties) in the type
> MediatorFactoryFinder is not applicable for the arguments (OMElement)
>
>
>
> The method getInstance() is undefined for the type ServerManager
>
> DataSourceInformation cannot be resolved
>
> DataSourceRegistrar cannot be resolved
>
> JNDIBasedDataSourceRegistry cannot be resolved
>
> MiscellaneousUtil cannot be resolved
>
> RMIRegistryController
>
> The import org.apache.synapse.util.datasource
>
> The import org.apache.synapse.util.MiscellaneousUtil
>
> The import org.apache.synapse.util.RMIRegistryController
>
>
>
> These are documented generally under the API changes section.
>
>
>
> I am yet to carefully go through your other suggestions on the API changes,
> let me get back to you on that ASAP.
>
>
>
> Thanks,
>
> Ruwan
>
>
>
>
>
> All the above seems to be stuff which had never been designed for external
> usage. It should not be hard to figure out how to properly replace it. All
> but one compile errors are from test/mock code used for verification of
> correctness of mediator implementations (including factories/serializers).
>
>
>
> If you guys are not in hurry tomorrow I would like to:
>
> -       fix the remaining issues in our custom code
>
> -       use a fixed version of the migration tool to migrate a large
> configuration file (for my initial test I used something really small)
>
> -       do some runtime mediation tests
>
>
>
> Unfortunately I will not find time today.
>
>
>
> Regards,
>
>    Eric
>
>
>
>
>   ------------------------------
>
> *From:* Hubert, Eric [mailto:Eric.Hubert@foxmobile.com]
> *Sent:* Thursday, December 09, 2010 11:44 AM
> *To:* dev@synapse.apache.org
> *Subject:* RE: [VOTE] Release Synapse-2.0.0 (Take2)
>
>
>
> Answers inline. Not every statement was meant to describe a „problem“, I
> simply always described what I did and what happened. Unexpected behaviour
> is separately mentioned…
>   ------------------------------
>
>
>
> 1) Synapse startup test with custom 1.2 config
>
> - config has been put into repository/conf/synapse-config as it seems to
> got ignored in repository/conf
>
>
>
> I think this is by design and we need to document this on the Upgrading
> guide.
>
> Agreed.
>
>
>
>   - Synapse does not startup due to a problem in the config (e.g. missing
> registry implementation class on the classpath)
>
>
>
> I will have a look at this, I guess this needs to be fixed if there is an
> issue, can you please give me a small config bit to re-produce this issue?
> may be you are using a WSO2 ESB registry class shipped with WSO2 which of
> course is not available in synapse. :-(
>
> Yes, my aim was to test a failure case. So it was absolutely expected that
> this fails. No issue at all!
>
>
>
>   - Unexpected behaviour: Although Synapse detects the problems, tries to
> perform a clean shutdown, it “keeps hanging” and does not return to the
> shell with an error return value
>
>
>
> Can you please attach the log for this and steps to re-produce, this again
> I would like to fix depending on the complexity of the issue... and if this
> gets slipped from 2.0.0 I suggest immediately spinning a 2.0.1 to fix this
> and any other this sort of issues from the 2.0.0 branch. WDYT?
>
> Yes, I personally do not consider this critical – it is simply only not
> nice. By the way, the same condition can be reached with many different
> issues in a user’s config. Reproduction is easy. Just specify any class name
> non existing in the classpath e.g. as the registry provider implementation.
>
>
>
> 2) Migration Tool
>
> - executing the migration tool expects a config in repository/conf (former
> config location)
>
>
>
> If you type help for the migration tool sh you will find that it is the
> default location the script looks for but you can specify your own location
> too.
>
> Maybe it would be slightly better if an no arg execution outputs a usage
> instead of assuming the default location and immediately starting to do
> something. But this is obviously a matter of taste…
>
>
>
>   - old copy copied there and restarted
>
> - migration tool modifies config
>
> - Unexpected behaviour:
>
> - after migration config stays in repository/conf and needs to be manually
> copied to repository/conf/synapse-conf to be recognized
>
>
>
> This again is the default location, assuming that you are running the
> migration tool for an old configuration, but you could of course give the
> new location to be saved after migrating the configuration. I guess we need
> a little bit more documentation around the migration tool.
>
>
>
>   - migration tool mistakenly removes namespaces (destroying the config),
> e.g.
>     <code xmlns:tns="http://www.w3.org/2003/05/soap-envelope"
> value="tns:Receiver"/> à <syn:code value="tns:Receiver" /> resulting in
> startup issues
>
>
>
> This we need to fix for this release, I will work on this.
>
> Great, I’d also consider this to be a blocker as the migration tool can
> convert a working config into a non-working one. I have not checked whether
> a backup is saved somewhere…
>
>
>
>   - migration tool removes indentation at the beginning of each xml
> element
>
>
>
> This is a known issue, but I agree needs to be fixed, since it is not
> critical I would live with this for the 2.0.0, but definitely a candidate
> for the next version, so we need to raise an issue ticket on the synapse
> JIRA for this.
>
>  Agreed, if no one beats me I can do this later today…
>
>
>
>
>
> 3) Traditional config in single file versus multi file configuration
>
> - Unexpected behaviour:
>
>   - replacing dummy synapse.xml with old (converted) config is not enough,
> it results in errors if main and/or fault sequences are used (as the must
> exist only once), sequence files need to be removed
>
>   from subdirectories
>
>
>
> Yes, this I agree with, but cannot do much I guess again need to explain
> this on the Upgrading guide
>
> Is the reason for this, that no concept has precedence over the other? One
> can mix both approaches as desired? If so I fully understand, but
> documentation is the least we should do. I would also vote for a prominent
> link to the Upgrading section in the documentation right from the front
> page. May under what’s new or so…
>
>
>
> 4) Usage of custom mediators / Site Documentation “Upgrading”
>
> - In the docu I could not quickly locate a document summarizing the steps
> which need to be done to upgrade custom mediators according to API changes
> (I received some AbstractMethodError). I did not check the mediator sources
> against the current libraries to find out what is no longer compiling….
> Anyway a quick summary of API changes would be nice. As long as I haven’t
> check the points which do not compile I cannot say for sure, whether those
> problems are due to the fact that public methods not considered to be part
> of the public API have been used on our end (which is not unlikely at all).
>
>
>
> I agree, will try to add more on the API changes, at least for the mediator
> API.
>
>  Great.
>
>
>
>
>   ------------------------------
>
> *From:* Ruwan Linton [mailto:ruwan.linton@gmail.com]
> *Sent:* Thursday, December 09, 2010 7:04 AM
>
>
> *To:* dev@synapse.apache.org
> *Subject:* Re: [VOTE] Release Synapse-2.0.0 (Take2)
>
>
>
> So the Sandesha module can be easily removed if you are not doing any
> reliable messaging stuff. Just need to delete the file from the
> repository/modules directory. I would even remove this error message from
> the custom build of Sandesha2 as we any way seem to go for the 3rd round of
> voting. :-)
>
>
>
> Eric, I would like to wait for your feedback to do the 3rd RC. So take your
> time, but report us any critical issue as soon as you get to them. May not
> need to be the complete list, you can report them one by one as and when you
> find those, so that we can fix them if needs to be and be ready for your
> next round of the feedback. BTW: must say that we really appreciate your
> feedback.
>
>
>
> Thanks,
>
> Ruwan
>
>
>
>
> --
> Ruwan Linton
> Software Architect & Product Manager, WSO2 ESB; http://wso2.org/esb
> WSO2 Inc.; http://wso2.org
>
> Lean . Enterprise . Middleware
>
> phone: +1 408 754 7388 ext 51789
> email: ruwan@wso2.com; cell: +94 77 341 3097
> blog: http://blog.ruwan.org
> linkedin: http://www.linkedin.com/in/ruwanlinton
> google: http://www.google.com/profiles/ruwan.linton
> tweet: http://twitter.com/ruwanlinton
>
>
>
>
> --
> Ruwan Linton
> Software Architect & Product Manager, WSO2 ESB; http://wso2.org/esb
> WSO2 Inc.; http://wso2.org
>
> Lean . Enterprise . Middleware
>
> phone: +1 408 754 7388 ext 51789
> email: ruwan@wso2.com; cell: +94 77 341 3097
> blog: http://blog.ruwan.org
> linkedin: http://www.linkedin.com/in/ruwanlinton
> google: http://www.google.com/profiles/ruwan.linton
> tweet: http://twitter.com/ruwanlinton
>



-- 
Ruwan Linton
Software Architect & Product Manager, WSO2 ESB; http://wso2.org/esb
WSO2 Inc.; http://wso2.org

Lean . Enterprise . Middleware

phone: +1 408 754 7388 ext 51789
email: ruwan@wso2.com; cell: +94 77 341 3097
blog: http://blog.ruwan.org
linkedin: http://www.linkedin.com/in/ruwanlinton
google: http://www.google.com/profiles/ruwan.linton
tweet: http://twitter.com/ruwanlinton

Mime
View raw message