[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2019-02-11 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-462337097
 
 
   framework/managed-context uses slf4j and is not dependend on utils. This 
needs to be solved by
   1. isolating logging in its own project and make it second to checkstyle
   1. exclude managed context from the standard use of logging


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2019-02-08 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-461787365
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2019-02-08 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-461781633
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2019-02-07 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-461505256
 
 
   used a addepted version of @rafaelweingartner's script above : 
   ```
   #!/bin/bash
   CLOUDSTACK_FOLDER="."
   
   ALL_CLOUDSTACK_CLASSES=$(find $CLOUDSTACK_FOLDER -name "*.java")
   
   for class in $ALL_CLOUDSTACK_CLASSES
   do
 count=$((count+1))
 echo "Processing ($count) class: $class"
   
 echo "Replacing org.apache.log4j.Level with 
org.apache.cloudstack.utils.log.Level"
 sed -i .loch 
's/org.apache.log4j.Level/org.apache.cloudstack.utils.log.Level/g' "$class"
   
 echo "Replacing org.apache.log4j.Logger with 
org.apache.cloudstack.utils.log.Logger"
 sed -i .loch 
's/org.apache.log4j.Logger/org.apache.cloudstack.utils.log.Logger/g' "$class"
   
 echo "Replacing java.util.logging.Logger with 
org.apache.cloudstack.utils.log.Logger"
 sed -i .loch 
's/java.util.logging.Logger/org.apache.cloudstack.utils.log.Logger/g' "$class"
   
 echo "Adding import to org.apache.cloudstack.utils.log.LogFactory"
 sed -i .loch '/import org.apache.cloudstack.utils.log.Logger/a \
   import org.apache.cloudstack.utils.log.LogFactory;
   ' "$class"
   
 echo "Replacing Logger.getLogger with LogFactory.getLogger"
 sed -i .loch 's/Logger.getLogger/LogFactory.getLogger/g' "$class"
   
 echo "Replacing org.slf4j.Logger with 
org.apache.cloudstack.utils.log.Logger"
 sed -i .loch 's/org.slf4j.Logger/org.apache.cloudstack.utils.log.Logger/g' 
"$class"
   
 echo "Replacing org.slf4j.LoggerFactory with 
org.apache.cloudstack.utils.log.LogFactory"
 sed -i .loch 
's/org.slf4j.LoggerFactory/org.apache.cloudstack.utils.log.LogFactory/g' 
"$class"
   
 echo "Replacing s_logger with LOG"
 sed -i .loch 's/s_logger/LOG/g' "$class"
   done
   ```
   the above scripts leaves some to be desired like:
   # I don't have a constructor taking a String, yet. there where only a 
handful of those and all used a Class.getname, so I converted the calls instead.
   # on occasion the script led to a double insertion of the factory import. 
fixed those by hand as well


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2019-01-29 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-458615431
 
 
   @rafaelweingartner I saw you IM comment and will look into what a spring 
suit would mean for logging. more later


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2019-01-29 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-458525484
 
 
   ok, then let me urge to give this priority. We are using antiquated 
libraries, as in out of service -, out of support libraries and moving forward 
will not get easier over time.
   
   If you method is the scripting search and replace of all methods used, I am 
afraid this is just a one of solution and not an architectural pattern for 
future stability. Please enlighten me because indeed I do not understand the 
alternative to be viable.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2019-01-29 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-458521892
 
 
   > @rafaelweingartner I would like to merge this and build on it.
   
   > @DaanHoogland I have not had free time to work on these things lately.
   
   I am making time to build on this and am not asking you to. We do need to 
agree on the way forward, though.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2019-01-29 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-458521354
 
 
   > I also do not see the benefits of this PR.
   It makes no sense not isolating dependencies and reducing surface area and 
this is just the most trivial example of that. I have given the benefits of 
such a way of working. What part do you disagree on?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2019-01-29 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-458520776
 
 
   > But, besides that, master is on freeze, and we are working to release 
4.12. Therefore, this PR has to wait in one way or the other.
   
   It was in way before the freeze so I don't see why it has to wait.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2019-01-29 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-458476655
 
 
   @rafaelweingartner I would like to merge this and build on it. I have not 
seen any follow up. can you expand or comply? /cc @GabrielBrascher 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-19 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-439816552
 
 
   @rafaelweingartner , can you enter something of a PoC PR so we can discuss 
further?
   Or sugest changes to this one?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-16 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-439386896
 
 
   so basically you are re-doing my work from #2276 but without reducing the 
surface area. I do not like it much so far.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-16 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-439378952
 
 
   > Anyways, to upgrade the log4j is not as a Dantesc task as you are 
portraiting. I am almost done, and I have almost had no time to work on it. I 
can finish, and then show you for us to discuss if you have an open mind.
   If you unify the log framework usage as well, I am open to your solution.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-16 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-439316964
 
 
   Also, @rafaelweingartner, I will -1 a PR that 'just' upgrades a structurally 
changed dependency, if it fails to makes efforts to isolate the dependency as 
described. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-16 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-439316167
 
 
   tl;dr you are not giving any argument to not use this strategy, you are only 
saying why this PoC is just a PoC.
   
   What you are describing is very elaborate as well, @rafaelweingartner. The 
script you describe is error prone. Not all calls and declaration are formatted 
the same way and the expressions look a bit to simple.
   
   I have seen the attempt to use slf4j in the console proxy code and it needs 
to be addressed as well. It is in the wrong place and incomplete.
   
   you say "For instance, your proposal could be applied to JPA ... as well, 
will we do that (when we move towards Spring-data and a real JPA 
implementation)? I hope not…". I do hope so, and I think you should as well, 
but this will be very challenging in the DAO case. I think it would be great if 
we could.
   
   I think we should definitely do this for a lot of external libraries, not 
reinventing the wheel but abstracting out and isolating dependencies on 
external eccentricities of those libraries, like call syntax/sematics. As you 
can see in my description of the issue (#2991) I am not being religious. I 
exclude (not definitely but as example) the DAO framework, as indeed spring 
data may make it more convenient to go about it differently. But NOTE: this is 
a temptation, luring us into a tie into the specifics of a framework we decide 
to commit ourself into in such a way.
   
   As you can see in #987 and #2276 and, I am sure numerous upgrade attempts by 
others as well, what you describe is not as simple as you say without isolating 
the use of an external library.
   
   I will follow up on this PR given mandate. we need to reduce the internal 
exposure to external libraries, not reinventing the wheel but isolating there 
use in proxy libraries. Your solution to upgrading log4j is not unifying 
logging framework use but only upgrading log4j as a one off solution 
essentially perpetuating the problem.
   
   In short, this PoC in itself will not solve the problem. it needs follow up 
and I dedicate myself to that. I do not want a PR like #2276, with 1710 files 
changed if a dependency decides to change a signature or a exposed structure. 
This will disable merging forward and hinder regular maintenance.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-15 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-439025126
 
 
   the travis build is successful but still reports an error here. I will 
extend this PoC somewhat to see what happened there.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-15 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-439024747
 
 
   @rafaelweingartner, I have no idea why someone would not want to do this. It 
is a best practice and in my opinion a blocker to upgrading log4j and any other 
logging frameworks. Please give me any arguments not to do this. I am pretty 
sure I can counter them.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-14 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-438705975
 
 
   ok, you will fail, but I will be patient for another day.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-14 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-438702885
 
 
   This is not a feature, @rafaelweingartner . It is a way to make the system a 
bit more maintainable and deal with technical debt.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-14 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-438613228
 
 
   ok, apart from @rhtyd 's request for the indentation correction I will merge 
this as is and spend part of my live on following up on this ;) I will be 
making small iteration of at most per project PRs, adapting the log wrapper to 
what it needs. next will be the utility project itself. thanks all.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-12 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-438155012
 
 
   hm, there where packages for the latest commit already. test had been run as 
well.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-06 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-436326092
 
 
   happy to work with you on what you need, @csquire . We do want to implement 
a large enough subset so all are served. ATM we have a maintenance problem, 
removal of which is our first objective. If you add an issue for your 
requirements I will add it if my puny abilities allow. Do not see the 
implementation of this PoC as the final stage or full interface.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-06 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-436303885
 
 
   @csquire hearsay admittedly but apache logging folk say that log4j2 is 
better faster neater than slf4j and when we consolidated the util package we 
can decide to wrap slf4j in it as well, right now slf4j is used at some places 
in the system, java logging at others and log4j version 1 mostly. I want to 
consolidate all logging after merging this PoC first and than upgrade to 
whatever we decide. The idea here is that we have only one place where the 
version of the 3rd party lib matters.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-02 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-435416639
 
 
   @rhtyd @rafaelweingartner @wido @swill @mike-tutkowski any comments?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-02 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-435304562
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] DaanHoogland commented on issue #2992: PoC for log library surface reduction (2991)

2018-11-01 Thread GitBox
DaanHoogland commented on issue #2992: PoC for log library surface reduction 
(2991)
URL: https://github.com/apache/cloudstack/pull/2992#issuecomment-435025739
 
 
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services