Enable delfin installer to run multiple instances of delfin components #625
Conversation
AmitRoushan
force-pushed
the
multi-instance-inst
branch
from
07bf6c4
to
2410b4b
Compare
Codecov Report
@@ Coverage Diff @@
## master #625 +/- ##
=======================================
Coverage 71.37% 71.37%
=======================================
Files 144 144
Lines 13077 13077
Branches 1563 1563
=======================================
Hits 9334 9334
Misses 3211 3211
Partials 532 532 |
|
Corresponding tear down of all the instances during uninstall , is this present ? |
|
@rajat-soda tear down is present already. It kills all the process by grepping delfin name and remove installed files |
AmitRoushan
changed the title
| ``` | ||
|
|
||
| Note: Multiple instances of exporter and api is not allowed currently. | ||
|
|
There was a problem hiding this comment.
Somewhere we can mention about the scope of this step is for single node, multi instances of processes
| for instance in range(0, instances): | ||
| proc_path = os.path.join(delfin_source_path, 'delfin', 'cmd', | ||
| process + '.py') | ||
| command = 'python3 ' + proc_path + ' --config-file ' + \ |
There was a problem hiding this comment.
Its always good to put the absolute path of the command (viz. /usr/bin/python3) and it should be constant driven
installer/install_delfin.py
Outdated
| # Ignore multiple instance of api | ||
| if process != 'api': | ||
| instances = os.environ.get(env_var) | ||
| if not instances: |
There was a problem hiding this comment.
do we need this? start_process arg default value is 1
There was a problem hiding this comment.
The reason the check is required because instances is type casted into int before passing to start_process. otherwise it would give NoneType
There was a problem hiding this comment.
Optimized the code a little bit based on suggestion
AmitRoushan
force-pushed
the
multi-instance-inst
branch
from
20f55a4
to
79b22dc
Compare
AmitRoushan
requested review from
NajmudheenCT,
joseph-v,
sushanthakumar and
kumarashit
What this PR does / why we need it:
The PR include changes to enable delfin installer for multi instance deployment of delfin components.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Installation documents are also updated