search

tango-controls / JTango

TANGO kernel Java implementation. JTango moved to https://gitlab.com/tango-controls/JTango
http://tango-controls.org
8 stars 14 forks source link

Fix #30 #33

Closed Ingvord closed 7 years ago

Ingvord commented 7 years ago

This PR is to fix #30 by replacing DbDevExportInfo with DeviceExportInfo from JTangoClientLang

Ingvord commented 7 years ago

@bourtemb Could you please test this fix? To build the artifact just clone this branch and execute mvn clean package in the root directory. As the result there must be JTango-9.3.3-SNAPSHOT-shaded.jar file in the assembly/target folder.

bourtemb commented 7 years ago

Thanks again for the work done on this issue. This version seems to solve the issue (https://github.com/tango-controls/TangoDatabase/issues/4) related to removing startup level info from astor. Having "null" only for the PID parameter would have been sufficient. Actually I don't understand why the ClassName parameter from DeviceExportInfo is included in the DevVarStringArray argin passed to DbExportDevice command. This className parameter is not taken into account/expected by the Database server. It currently does not have any impact on the behaviour of the Database server but it is not very clean to transfer unnecessary data.

I guess the dependency to google guava is no longer necessary. It seemed to be used only for the export_server() method which seem to be obsolete. There is no (longer) DbExportServer command on the Database server, unless there are other Tango Database servers implementations I am not aware of.

Ingvord commented 7 years ago

I see. We have to look through Tango DB server and see if there something else to remove.

I will remove guava as for now there is indeed only one place where it is used. The method will throw an exception.

Code cleanup will be done in a separate PR.

Ingvord commented 7 years ago

@Pascal-Verdier could you please review this PR?

codecov[bot] commented 7 years ago

Codecov Report

Merging #33 into master will increase coverage by 0.2%. The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #33     +/-   ##
=========================================
+ Coverage   10.03%   10.24%   +0.2%     
=========================================
  Files         657      657             
  Lines       51763    51751     -12     
  Branches     7328     7328             
=========================================
+ Hits         5195     5300    +105     
+ Misses      46067    45926    -141     
- Partials      501      525     +24
Impacted Files Coverage Δ
common/src/main/java/fr/esrf/TangoApi/ApiUtil.java 0% <ø> (ø) :arrow_up:
...a/org/tango/client/database/cache/ServerCache.java 0% <ø> (ø)
...org/tango/client/database/cache/DatabaseCache.java 0% <ø> (ø)
...ommon/src/main/java/fr/esrf/TangoApi/Database.java 0% <ø> (ø) :arrow_up:
...va/org/tango/client/database/DeviceImportInfo.java 0% <ø> (ø)
...va/org/tango/client/database/DeviceExportInfo.java 0% <ø> (ø)
...g/tango/client/database/cache/NoCacheDatabase.java 0% <ø> (ø)
...in/java/org/tango/client/database/FileTangoDB.java 58.08% <ø> (ø)
...va/org/tango/client/database/cache/ClassCache.java 0% <ø> (ø)
...rc/main/java/fr/esrf/TangoApi/DbDevImportInfo.java 0% <ø> (ø) :arrow_up:
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 49fb6a5...04017bc. Read the comment docs.

Pascal-Verdier commented 7 years ago

Hi 27 modified files. A bit too much to have a clear idea of what has really changed..... I'll do the PR