Unit tests for sds_sync module

[jmolmo]

sds_sync module implementation

In my opinion there is needed a refactorization of the code in this module. Basically the problem is the "extension" of the implementation in each of the methods/functions.
It seems that all the execution units can be easily simplified "breaking" them in small auxiliary functions.
This will give us several advantages:

1. More easy to read/maintain the code
2. Unit test more easy to do, and a real possibility of test diferent scenes in the unit tests, not only code coverage is important.
3. More easy to extend/add functionality
4. More difficult to repeat things already done/checked in the same function

Method GlusterIntegrationSdsSyncStateThread.Run()

Cluster state and cluster node context are loaded several times in different parts of the method.
I think that even in the case that the situation/state of the cluster change while this method is being executed is not good to load again this information.
IMHO, this can lead to inconsistences. I think that it would be safer to load the information at the beginning and use this information in the rest of the method. Periodic syncs will give us the evolution of the state, but refreshing the status information while we are executing the "run" method seems to be dangerous.

I haven't got a good knowledgment of the whole system, and i haven't work with gluster clusters, also not time enough to solve this lacks, so it is possible that i be wrong in my changes, in any case, this is nothing that cannot be solved with a good code review!, and i really think that this changes will improve the code.

Function check_volumes()

This function has been extracted (refactorization) from the GlusterIntegrationSdsSyncStateThread.Run(). The unit test shows that each time a new option is found, it is saved ... probably it can be modified to save only in one attemp all the options.

Another thing i do not unsderstand is why if there is no options in the volume loaded, the options discovered (gluster cmd details) are not included in the volume.

[centos-ci]

Can one of the admins verify this patch?

[centos-ci]

Can one of the admins verify this patch?

[r0h4n]

@GowthamShanmugam @anmolsachan please review this

@shtripat The changes seem interesting, please review them

[shtripat]

Overall changes looks fine to me.
There is a minor comment.

[codecov]

Codecov Report

Merging #661 into master will increase coverage by 3.38%.
The diff coverage is 68.06%.

@@            Coverage Diff             @@
##           master     #661      +/-   ##
==========================================
+ Coverage   38.76%   42.14%   +3.38%     
==========================================
  Files          42       42              
  Lines        2056     2088      +32     
  Branches      279      282       +3     
==========================================
+ Hits          797      880      +83     
+ Misses       1232     1174      -58     
- Partials       27       34       +7

Impacted Files	Coverage Δ
tendrl/gluster_integration/sds_sync/__init__.py	33.09% <68.06%> (+18.66%)	⬆️

---

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 78cbb58...4496163. Read the comment docs.

[shtripat]

Somehow even I am not able to see the comment. Anyway overall code changes look fine.

[jmolmo]

Thx!! No problem :-) El El vie, 15 jun 2018 a las 19:11, Shubhendu <notifications@github.com> escribió:

…

Somehow even I am not able to see the comment. Anyway overall code changes look fine. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#661 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvFkTJGcxDoRnffAt6xqsvlyfdMb9Xtks5t8-q2gaJpZM4UpHzd> .