-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add last used function #159
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should really be a function in ocflib.lab.stats
|
the functions in that module can/should have the ability to take credentials. splitting this functionality out like this without good reason breaks the logical design of ocflib. |
So probably I can put the code into that module and refractor a little bit so that when specified with some arguments it does not connect with anonymous username? This is not so easy though since in what step should we connect to the database? @abizer |
Look at how the other functions call |
Now I put those functions into stats.py with some slight modifications. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, just some small nits
This lgtm, to fix the last errors you should make sure to run |
|
||
|
||
def last_used(host): | ||
with open('/etc/ocfstats-ro.passwd', 'r') as fin: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ocflib shouldn't concern itself with getting credentials itself - it should be able to operate regardless of where the credentials are, i.e. these functions should operate by accepting an authenticated context object, e.g. shorturls.py, mail.py rather than by hardcoding the location of a file on disk, especially when this file isn't universally available.
There are still issues with this PR that ought to be fixed. |
I added a last used function since it would be useful if we can figure out who is the last user in cases such as (ocf/utils#96).
We can use
last_used(...)["end"] == None
to determine whether the user is currently using the machine.