getting os details with /api/os #22
No reviewers
Labels
No Label
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Sysdeploy/idlers-agent#22
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "shailaja/idlers-agent:os_details"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
@ -353,0 +355,4 @@
def get_os_id(self):
try:
response = urllib.request.urlopen(f"{self.host}/api/os")
This will not work. You should create a dedicated method in the ServerManager and use existing
send_request
.@ -353,0 +357,4 @@
try:
response = urllib.request.urlopen(f"{self.host}/api/os")
os_list = json.loads(response.read().decode())
current_os = os.uname().sysname.lower()
os.uname().sysname
provides the generic name of the operating system, not the specific distribution name or version.For example, it would return 'Linux' for any Linux distribution (like Ubuntu, Fedora, etc.), 'Darwin' for macOS, and so on.
We should get complete name, for example
Ubuntu 22.04.3 LTS
in case of test2. We should read from/etc/os-release
file.@ -353,0 +361,4 @@
for os_entry in os_list:
if current_os in os_entry['name'].lower():
return os_entry['id']
return next(os['id'] for os in os_list if os['name'].lower() == "other")
Instead of directly falling to "other", maybe we can use id of ubuntu if exact name not found and if it's ubuntu, similarly for other OSes. For final fallback we can have "other" or "custom".
54f87b0314
todf525d2908
276f29c707
to04671ba669
34e2a03db8
to90dcb330a4
@ -240,0 +248,4 @@
if not os_info:
logging.error("No OS release info found.")
return next(os['id'] for os in os_list if os['name'].lower() == "other" or os['name'].lower() == "custom")
current_os = f"{os_info.get('NAME', 'Unknown')} {os_info.get('VERSION', '').strip()}".strip().lower()
As discussed, we should use a combination of
id
andversionId
. Check the exact key name in the file.@ -239,1 +236,3 @@
return os_id
def get_os_release_info(self):
try:
with open('/etc/os-release') as f:
Since
/etc/os-release
doesn't exist on openwrt, we should fallback to/etc/VERSION
.As shared in the skype:
From
/etc/VERSION
, we can use a combination ofos_name
andproductversion
.@ -240,0 +261,4 @@
return next(os['id'] for os in os_list if os['name'].lower() == "other" or os['name'].lower() == "custom")
except Exception as e:
logging.error("Failed to fetch OS ID: {}".format(e))
return 27
Instead of returning a hard-coded nnumber, we'll return "other" or "custom" id
@ -408,0 +433,4 @@
def get_os_list(self):
try:
connection = http.client.HTTPSConnection(self.host)
Again, we should use
send_request
here@ -240,2 +264,4 @@
return 27
def create_post_data(self):
Missing parameter for os list
Why is almost entry method is wrapped up in try-except? Please list down the cases where you think it'll break inside the
try
and this try-except will be useful.@ -243,3 +269,3 @@
post_data = {
"server_type": 1,
"os_id": self.get_os(),
"os_id": self.get_os(os_list),
I think you mean
get_os_id
4a3222bb84
to9c581fb6dd
@ -240,0 +271,4 @@
for os_entry in os_list:
if os_entry['name'].lower() in ["other", "custom"]:
return os_entry['id']
return None
os id cannot be none, so at least this method should never return a None. Also, this case should never practically happen, even if it happens, you can return 1.
@ -240,0 +274,4 @@
return None
if 'NAME' in os_info and 'VERSION' in os_info:
current_os = f"{os_info.get('NAME', 'Unknown')} {os_info.get('VERSION', '').strip()}".strip().lower()
It's still not using
id
andversionId
@shailaja We want to use
ID
andVERSION_ID
instead ofNAME
andVERSION
, then you don't need tolower()
that.You also need to modify this line to use ID and VERSION_ID.
Also we should refrain from using f-string anywhere in the new code.
@ -240,2 +304,4 @@
return None
def create_post_data(self):
Missing parameter for os list.
@ -407,0 +473,4 @@
def get_os_list(self):
os_list = self.send_request('GET', '/api/v1/os')
if os_list:
Don't need to do if-else. can be one-liner like other
get_*
methods.49a3f63c34
to777d742d40
@ -240,3 +301,4 @@
def create_post_data(self):
ram, disk = self.get_ram_and_disk()
os_list = self.get_os_list()
get_os_list
method does not exist in theServerData
class. You need to takeos_list
as an argument tocreate_post_data
@ -406,1 +468,4 @@
return self.create_note(note_data)
def get_os_list(self):
os_list = self.send_request('GET', '/api/v1/os')
The correct endpoint is
/api/os
and not/api/v1/os
334ce0aefd
to5316b0fb07
4480a3361f
toeec2530205
@ -242,0 +276,4 @@
if 'ID' in os_info and 'VERSION_ID' in os_info:
current_os = (os_info.get('ID', 'Unknown') + " " + os_info.get('VERSION_ID', '').strip()).strip()
else:
current_os = (os_info.get('os_name', 'Unknown') +" " os_info.get('productversion', '').strip()).strip().lower()
You missed a
+
after" "
31ee586a9b
todeac2cdee2
@ -428,2 +494,4 @@
logging.info('Server id: {}'.format(server_id))
os_list = server_manager.get_os_list()
post_data = server_data.create_post_data(os_list)
Why this call again? There's already a call for create_post_data above, you need to modify that.
deac2cdee2
tod6afa1c5eb
4fa1281c10
to8253e5fc51