Hey Igor,
Yeah, I can agree with that. I think the biggest problem is that the
dotcms codebase isn't as put together as ES. So fixing it this way
affected the least amount of code. I could attempt to restructure their
properties stuff, but there is quite a bit of code that would need to
change - I don't think they would go for it.
Honestly, the upgrade would take far too much work for my client to be
willing to spend the time - especially since we have a work around and a
possible fix already. Dotcms would have to do the work to change that, but
they are still using Struts 1 and JUST upgraded to Spring 3 from 2. Not to
say that those things don't make sense for them, but that it takes forever
for them to do anything large - the codebase just isn't set up for rapid
development. Also, there is a very large plugin community that could be
affected - further lengthening the time for a fix - if in fact systemic
changes are required. We were hoping to be able to have an instance set up
in December sometime.
So to sum up, I agree that their properties framework needs work. I also
agree that it doesn't make sense to try and put a fix in place that is out
of place. However, having said all that - I also think that there are
plenty of environments that coders have to deal with that limit integration
options - out of lack of foresight or other issues. Giving an ES
TransportClient the option to not load ANY configurations outside of the
ones passed in the Settings object is, I think, reasonable. There are
just too many legacy and/or crappy applications to go around fixing all of
them to be what you or I would expect or want. Our ES plugin predates the
use of ES internally and as a consultant, it is hard to justify that sort
of additional expenditure. Please let me know if you have any questions or
additional concerns.
Thanks for your time!
Nick
Thanks!
Nicholas Padilla
www.monstersoftwarellc.com
“Problems cannot be solved by the same level of thinking that created them.”
“Learn from yesterday, live for today, hope for tomorrow. The important
thing is not to stop
questioning.http://thinkexist.com/quotation/learn_from_yesterday-live_for_today-hope_for/222120.html
”
Albert Einstein http://thinkexist.com/quotes/albert_einstein/
On Mon, Nov 26, 2012 at 10:42 AM, Igor Motov imotov@gmail.com wrote:
Hi Nicholas,
I agree that it sounds more like a workaround. What I am still struggling
with is figuring out if this is a workaround for an elasticsearch issue or
a dotcms issue. It's easy to add this feature to elasticsearch. But before
we do that, we should ask ourselves a question: Are we solving the right
problem? Elasticsearch already provides several different ways of supplying
configuration. The dotcms application is using the most global way that
overrides settings for all es instances running in the dotcms process and
that affects your use case. The problem can be fixed on the dotcms side by
changing the default configuration files that dotcms is shipping with, or
even better by switching to a config file base configuration instead of
system properties base configuration. So, would it make more sense to fix
it in dotcms rather than add a new feature to elasticsearch? I am not very
familiar with dotcms, so I might be missing something important here. What
do you think?
Igor
On Monday, November 26, 2012 11:02:39 AM UTC-5, Nicholas Padilla wrote:
Hey Igor,
I have found that this does work, however the .properties needs to be a
.yml and in the YAML format. This is a workaround and not a final
solution, as it makes upgrading more difficult due to the changing of the
core .properties file. As a dotcms best practice, everything should be
done to customize the application in a config plugin. This is so the
dotcms team can make updates to these files as needed, bug fixes or new
features. This could be a solution if the parent architecture were a bit
different. I have to pass this along with some notes as to how to upgrade,
when you start throwing more cut and paste stuff in there the more room for
error there is.
Thanks for all your help on this matter! We at least have a stop gap
measure in case the code adjustment I added takes a long time to get
committed.
Thanks again!
On Friday, October 26, 2012 1:10:35 PM UTC-6, Igor Motov wrote:
But where are they adding settings to the System Properties from? Isn't
it dotCMS/WEB-INF/classes/dotm**arketing-config.properties? So, if you
will move properties from this file to an external file, as I described
above, the only property that they will add to System Properties will be
es.config= which points to the location of the config file. As a result,
the embedded server will load all properties that it needs from the config
file, while your client will not simply ignore this config file and will
use only properties that you specified.
On Fri, Oct 26, 2012 at 3:04 PM, Nicholas Padilla <
nich...@monstersoftwarellc.**com> wrote:
Hey Igor!
Thanks for the reply! I had thought about that but after looking at
how dotcms loads it properties I found I couldn't move this direction.
They are adding all properties to the System Properties. Creating a new
TransportClient using the constructor you referenced only restricts from
loading settings from the Classpath. They are adding all settings to the
System Properties and that the TransportClient, by default, loads and
overrides my Settings object from the System Properties. This is where
the problem lies, there isn't a way to tell the TransportClient to NOT load
properties from the System Properties at this point. That is what my patch
does, allows you to say to ONLY use settings applied from the Settings
object.
Thanks!
Nicholas Padilla
www.monstersoftwarellc.com
“Problems cannot be solved by the same level of thinking that created
them.”
“Learn from yesterday, live for today, hope for tomorrow. The
important thing is not to stop questioning.http://thinkexist.com/quotation/learn_from_yesterday-live_for_today-hope_for/222120.html
”
Albert Einstein http://thinkexist.com/quotes/albert_einstein/
On Fri, Oct 26, 2012 at 12:57 PM, Igor Motov imo...@gmail.com wrote:
Nicholas,
Here is what you could try:
Move all elasticsearch properties from dotCMS/WEB-INF/classes/**dotmarketing-config.properties
to another file, for example, myes.properties
Replace all elasticsearch properties in dotCMS/WEB-INF/classes/**dotmarketing-config.properties
with the following single line that points to the myes.properties file.
es.config=path/to/myes.**properties
Start your client with loadConfigSettings set to false:
This way embedded elasticsearch in dotcms should get its settings
from myes.properties, but the client will ignore them if you will set
loadConfigSettings to false:
... = new TransportClient(**settingsBuilder, false);
I am not sure how dotcms instantiates embedded **elasticsearch node,
but if they didn't change defaults, it might work.
On Tuesday, October 9, 2012 5:32:02 PM UTC-4, Nicholas Padilla wrote:
Hello,
I have submitted a pull request here: https://github.com/**elast**
icsearch/elasticsearch/**pull/**2312https://github.com/elasticsearch/elasticsearch/pull/2312
Thanks!
On Monday, October 8, 2012 8:49:07 AM UTC-6, Drew Raines wrote:
Nicholas Padilla wrote:
I finally started to look into the codebase to see if there was
anything stood out. I found that the Settings provided to the
TransportClient constructor can be overridden by System
Properties.
With some testing I found that this was indeed the problem. I
have
been working with 0.19 branch and have some small changes that
will
provide this functionality without breaking the contract with
other
users. Should I commit this to both the 0.19 and master or just
master?
Commit it to master and create a pull request. Until we see a patch
we can't discuss what you're proposing to change.
-Drew
--
--
--
--