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 EOL API #2117
Add EOL API #2117
Conversation
webmin/os-eol-lib.pl
Outdated
year => $eol_date->{'year'}, | ||
short => $eol_date->{'short'}, | ||
timestamp => $eol_timestamp }; | ||
$eol_data->{'_eol_in'} = |
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.
Why not just return a timestamp and have the caller of this API format it as they want?
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.
It's better call it once and make object just ready.
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.
An internal API like this shouldn't contain a lot of presentation logic - it should be up to the caller to format the dates for display.
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.
Agreed! I have factored this logic out. Nevertheless, there is still a lot to do to prevent multiple callers from having to reimplement this logic - there has to be a dedicated subroutine for this.
Alright, all fixed! |
Jamie, all done this time! Please have another look! |
webmin/os-eol.map
Outdated
@@ -0,0 +1,1810 @@ | |||
=pod |
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.
Is this just the file that the API provides?
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.
The API doesn't provide this. I made it as a reference to remember what the data structures are like and what Webmin thinks about OSes.
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.
So is this file used by any Webmin code? I'm confused as to why we need to check it in ..
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.
No, this is just a .map file. It won't go to the package itself. I checked it in for the reference!
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.
I think we should remove it, as it's kind of confusing.
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.
Alright, it's done. I just wanted to make it easier for future support.
Yeah, I know. But it's much more simple to compare miniserv server version, as it's always getting updated on Webmin upgrade. |
webmin/os-eol-lib.pl
Outdated
} | ||
|
||
# eol_update_cache() | ||
# Caches the use of EOL data for next 30 days |
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.
It's not 30 days anymore is it?
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.
It's not 30 days anymore is it?
It isn't, just an old comment. Fixed now.
Hey Jamie,
As discussed here I created an API to return an object of EOL related data for the current system.
Returned EOL object looks like this:
We can later use it in alert message, and table row to be show on the dashboard depending on configurable options.
The EOL data is cached for 5 days by default.
Have a look please!