Support sles 15 sp2 distro #2272
Conversation
nagworld9
requested review from
dhivyaganesan,
kevinclark19a,
larohra,
narrieta and
ZhidongPeng
as code owners
Codecov Report
@@ Coverage Diff @@
## develop #2272 +/- ##
===========================================
+ Coverage 70.80% 70.81% +0.01%
===========================================
Files 96 96
Lines 13857 13857
Branches 1979 1979
===========================================
+ Hits 9811 9813 +2
- Misses 3618 3619 +1
+ Partials 428 425 -3
Continue to review full report at Codecov.
|
| @@ -165,6 +165,13 @@ def get_data_files(name, version, fullname): # pylint: disable=R0912 | |||
| else: | |||
| # sles 12+ and openSUSE 13.2+ use systemd | |||
| set_systemd_files(data_files, dest=systemd_dir_path) | |||
| elif name == 'sles': # sles 15+ distro named as sles | |||
There was a problem hiding this comment.
have you confirmed this comment? -
sles 15+ distro named as sles
I mean what does suse 13,14,etc do? Do we even have those distros? It would be wise to check all this out before committing it to code.
There was a problem hiding this comment.
we don't have 13.x, 14.x versions and also, azure not endorsed those distros.
bin/version2/waagent
Outdated
| @@ -0,0 +1,53 @@ | |||
| #!/usr/bin/env python3 | |||
| # | |||
There was a problem hiding this comment.
i'd suggest naming the new directory 'py3' or something like that instead of 'v2'
setup.py
Outdated
| @@ -43,6 +43,10 @@ def set_bin_files(data_files, dest, src=None): | |||
| src = ["bin/waagent", "bin/waagent2.0"] | |||
| data_files.append((dest, src)) | |||
|
|
|||
| def set_bin_version2_files(data_files, dest, src=None): | |||
There was a problem hiding this comment.
we don't need this new function, do we? we can use set_bin_files
There was a problem hiding this comment.
Idea for separate function is we only set new binary file if that distro has py3 requirement rest we use set_bin_files.
There was a problem hiding this comment.
Keeping in one place required extra logic and need to know distro type again
There was a problem hiding this comment.
the only thing we are changing is the default value... am i missing something? instead of creating a new function with a different default value you could just call the original function with the different value.
There was a problem hiding this comment.
and we also need to copy waagent2.0
There was a problem hiding this comment.
Not only for SLES.. The setup was codded in way that we set binary files first irrespective of what distro it is.. If we want to call only one time then I would have to remove that and include all 10 if block statements..
There was a problem hiding this comment.
calling outside
Line 103 in f59375f
There was a problem hiding this comment.
I see. I looked at the rest of the code and it could be better abstracted, but for now calling set_bin_files with the correct files for each of the distros (just ad we call set_conf_files) would be clearer
There was a problem hiding this comment.
cool I will update that
Description
Issue # TASK 8742449
Add support for SLES 15 distro
DCR run
PR information
Quality of Code and Contribution Guidelines