> On 2011-09-27 13:05:14, Dan Dumont wrote: > > There's another fix to this problem that addresses the issue expressed by > > Doug Davies in the dev list. > > instead of forcing people to register an API_PATH for the common container, > > we should by default NOT register an api path if it is not explicitly > > provided. > > > > Let me know if you want to go after it, otherwise it's on my to do list. > > Ryan Baxter wrote: > +1 > > li xu wrote: > Dan, that's good information. Would you please point me to the fix or the > discussion thread of it? Sounds great if we don't need to force the API_PATH > registration. I'd like to understand it better and possibly help out. > However in the meantime I think we should handle these two patch > seperately. Registering API_PATH with common container is still a legal way > to handle different context root use case. This patch also includes other > minor clean up and features such as prefill gadget field with correct context > root. > thanks, > li > > > Dan Dumont wrote: > The thread in the dev list has the subject: Metadata > The code that should probably change is in features/container/service.js > where we use a default of '/rpc' as the api path. > We then register it later on, overriding the correctly registered > endpoint that gets injected in the container config. > > I'm thinking if the api path is not specified, we use the one that was > injected as the container config. I'm wary of not registering at all. You > could try that also to see if it works correctly. > > li xu wrote: > Thanks, Dan. I will help take a look, can work offline with you on this. > Seems to me it will be a nice feature to have. > > while we're working on a pending feature, could anyone please help review > and close this defect? thanks. >
Lee I don't think we should add the code that sets up the RPC path correctly if Dan's fix will handle that. While I agree what you are doing here is correct, I do not want to make the code overly complicated just as a temporary fix. I would rather us remove the code which deals with the RPC servlet path and leave the code for the prefilled gadget field and just commit that. Also can you please attach a JIRA to the review? - Ryan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2052/#review2086 ----------------------------------------------------------- On 2011-09-26 21:22:35, li xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2052/ > ----------------------------------------------------------- > > (Updated 2011-09-26 21:22:35) > > > Review request for shindig, Paul Lindner, Henry Saputra, and Ryan Baxter. > > > Summary > ------- > > The fix will set up API patch when common container is created in these > sample pages. > > var testConfig = testConfig || {}; > testConfig[osapi.container.ServiceConfig.API_PATH] = contextRoot + '/rpc'; > var CommonContainer = new osapi.container.Container(testConfig); > > also includes other minor cleanup. > > > Diffs > ----- > > > /trunk/content/samplecontainer/examples/conservcontainer/ConServContainer.js > 1176015 > /trunk/content/samplecontainer/examples/conservcontainer/index.html 1176015 > /trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js > 1176015 > > Diff: https://reviews.apache.org/r/2052/diff > > > Testing > ------- > > passed maven build. > > > Thanks, > > li > >
