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

Adding project.maintenance #396

Merged
merged 7 commits into from Aug 31, 2021
Merged

Adding project.maintenance #396

merged 7 commits into from Aug 31, 2021

Conversation

max-hassani
Copy link
Member

Introduces Maintenance class, which returns get_database_statistics as an attribute of a pyiron project. Of course, the maintenance class can be further extended to include other performance measures, like disk spaces, available memory, etc.

The use case is:

from pyiron_base import Project
pr = Project()
pr.maintenance.database_statistics

this returns a dataframe including measure about postgres database statistics.

@jan-janssen
Copy link
Member

Is not it sufficient to import directly? like:

from pyiron_base import get_database_statistics
get_database_statistics()

I do not see why the database statistics are related to a specific project.

@max-hassani
Copy link
Member Author

Is not it sufficient to import directly? like:

from pyiron_base import get_database_statistics
get_database_statistics()

I do not see why the database statistics are related to a specific project.

I agree! But Jörg required it yesterday in the technical meeting. I believe, he had the idea that the maintenance class can be extended to include more performance measures later on.

@jan-janssen
Copy link
Member

But then at least keep the same function name, rather than making it a property:

from pyiron_base import Project
pr = Project()
pr.maintenance.get_database_statistics()

Co-authored-by: Niklas Siemer <70580458+niklassiemer@users.noreply.github.com>
@niklassiemer
Copy link
Member

Is not it sufficient to import directly? like:

from pyiron_base import get_database_statistics
get_database_statistics()

I do not see why the database statistics are related to a specific project.

I agree! But Jörg required it yesterday in the technical meeting. I believe, he had the idea that the maintenance class can be extended to include more performance measures later on.

Yes, I raised the same argument that the database statistics is not related to a specific project. However, he really wants to be able to do everything with one import statement

from pyiron{_module} import Project

Therefore, also #385.

@max-hassani
Copy link
Member Author

But then at least keep the same function name, rather than making it a property:

from pyiron_base import Project
pr = Project()
pr.maintenance.get_database_statistics()

Now, it is implemented as you suggested.

@stale
Copy link

stale bot commented Aug 18, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 18, 2021
including pr.maintenance.global_status.get_database_statisitics()
@stale stale bot removed the stale label Aug 30, 2021
Co-authored-by: Niklas Siemer <70580458+niklassiemer@users.noreply.github.com>
@max-hassani
Copy link
Member Author

@niklassiemer, the reason that I chose self.global_status was because global is a keyword in python. Since the tests are failing too, I will revert it to global_status.

@niklassiemer
Copy link
Member

Sure, I never used the global and looking at this, I think I never will...

@max-hassani
Copy link
Member Author

@niklassiemer, if you agree, we can merge this.
I will open another pull request to include a unittest and improve the coverage. Currently, I do not know exactly if a mocked up database connection is sufficient, which I doubt, or a running postgress database via github workflows is necessary. I have to learn more about it.

@niklassiemer niklassiemer merged commit a84848d into master Aug 31, 2021
@delete-merged-branch delete-merged-branch bot deleted the project_maintenance branch August 31, 2021 11:07
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

3 participants