Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement status.uptime on macOS #37157

Closed
wants to merge 10 commits into from
Closed

Conversation

jfindlay
Copy link
Contributor

@jfindlay jfindlay commented Oct 21, 2016

What does this PR do?

Implement status.uptime on macOS

What issues does this PR fix or reference?

#37158

Tests written?

Yes

@cachedout
Copy link
Contributor

Go Go Jenkins!

Copy link
Contributor

@isbm isbm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfindlay please look at these.

elif salt.utils.is_sunos():
cmd = "kstat -p unix:0:system_misc:boot_time | nawk '{printf \"%d\\n\", srand()-$2}'"
ut_ret['seconds'] = int(__salt__['cmd.shell'](cmd, output_loglevel='trace').strip() or 0)
seconds = float(__salt__['cmd.shell'](cmd, output_loglevel='trace').strip() or 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Solaris, with kstat you will always get an integer, no need to float here.

raise CommandExecutionError('Cannot find kern.boottime system parameter')
sec_data, usec_data = bt_data.split('}')[0].strip(' {').split(', ')
sec = sec_data.split('sec = ')[1]
usec = usec_data.split('usec = ')[1]
Copy link
Contributor

@isbm isbm Oct 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not do that and suggest something more reliable, although probably less readable:

data = bt_data.split("{")[-1].split("}")[0].strip().replace(' ', '')
uptime = dict([(k_sec, int(v_sec,))
                for k_sec, v_sec in [pair.strip().split("=")
                                     for pair in data.split(",")]])

What you do here is to turn mac returned data into a dictionary, and refer to it something like:

uptime['usec']
uptime['sec']

You can turn the above to a lambda and also query sleep time etc.

P.S. It can be oneliner, but then @thatch45 will hate me for this. 😆

sec_data, usec_data = bt_data.split('}')[0].strip(' {').split(', ')
sec = sec_data.split('sec = ')[1]
usec = usec_data.split('usec = ')[1]
seconds = curr_seconds - float(sec + '.' + '{0:0>6}'.format(usec))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, Mac is also only an integer, no floats there. 😃

@isbm
Copy link
Contributor

isbm commented Oct 24, 2016

@cachedout please hold a moment. 😄

@cachedout
Copy link
Contributor

@isbm Will do. :]

@isbm
Copy link
Contributor

isbm commented Oct 24, 2016

@jfindlay two more to go. :-)

@cachedout
Copy link
Contributor

@jfindlay Could you please let me know where you are with this one?

@cachedout
Copy link
Contributor

Because there has been no response, I am closing this. @jfindlay when you are ready to go here please re-open it.

@cachedout cachedout closed this Oct 27, 2016
@jfindlay jfindlay reopened this Oct 27, 2016
Copy link
Contributor

@isbm isbm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfindlay Thanks for the update! Could you please still look at this one?

if not bt_data:
raise CommandExecutionError('Cannot find kern.boottime system parameter')
sec_data, usec_data = bt_data.split('}')[0].strip(' {').split(', ')
seconds = int(curr_seconds - int(sec_data.split('sec = ')[1]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfindlay If you could use my previous suggestion, we by now would also have sleep time of Mac!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically:

data = bt_data.split("{")[-1].split("}")[0].strip().replace(' ', '')
uptime = dict([(k_sec, int(v_sec,))
                for k_sec, v_sec in [pair.strip().split("=")
                                     for pair in data.split(",")]])

What you do here is to turn mac returned data into a dictionary, and refer to it something like:

uptime['usec']
uptime['sec']

You can turn the above to a lambda and also query sleep time etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you want me to do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Point is, you could reuse it and get also other values from the sysctl, such as "sleeptime" or "waketime". That said, you could turn the code above to a def _internal_function(data) and reuse it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isbm, you're welcome to open a pull request on my branch, https://github.com/jfindlay/salt/compare/mac_time...isbm:mac_time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, but that's easy:

def _parse_sysctl(data):
    data = data.split("{")[-1].split("}")[0].strip().replace(' ', '')
    return dict([(k_sec, int(v_sec,))
                    for k_sec, v_sec in [pair.strip().split("=")
                                         for pair in data.split(",")]])

...
seconds = int(curr_seconds - _parse_sysctl(bt_data)['sec'])

At the moment I don't have Salt installed on OSX.

@isbm isbm mentioned this pull request Oct 29, 2016
@isbm
Copy link
Contributor

isbm commented Oct 29, 2016

@jfindlay @cachedout I find #37321 superseding this one. Maybe it would be possible to collaborate with @sjorge ? He did really awesome job. I'd suggest @sjorge take a look at current proposed changes from here, cherry-pick @jfindlay's stuff and let's merge his PR instead.

@sjorge
Copy link
Contributor

sjorge commented Oct 29, 2016

@isbm ooh I did not know there was a sysctl module, merging stuff now, OSX == FreeBSD it seems.

So we have 4 big buckets: Linux, SunOS, FreeBSD+OSX, OpenBSD+NetBSD... let me try and fold @jfindlay stuff inside mine. (Done)

I'm looking at the unit test now.

@cachedout
Copy link
Contributor

I'm fine with using the PR from @sjorge if that's what's agreed on. Any objection, @jfindlay ?

@jfindlay
Copy link
Contributor Author

@cachedout, none from me. Excellent job everyone.

@jfindlay jfindlay closed this Oct 31, 2016
@jfindlay jfindlay deleted the mac_time branch November 1, 2016 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants