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

Add job list page #657

Merged
merged 14 commits into from
Apr 24, 2024

Conversation

superstar54
Copy link
Member

@superstar54 superstar54 commented Apr 3, 2024

To help users easily search and manage their jobs, this PR adds a new job list page. This is the first step to fix #515 .

Home page

On the home page, I added a new Utils section on the right side of the logo. Currently, there is only the job list item, but we can add more items in the future, e.g., 'plugin list' in #646 .

Job list page

  • show a table of the processes: pk, ctime, state, relaxed, properties
  • delete button and inspect button.

Demo

qeapp-joblist.mp4

Future improvement

After this PR, open issues on

  • add search bar

Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.47%. Comparing base (f63da7d) to head (6e57bc5).
Report is 58 commits behind head on main.

❗ Current head 6e57bc5 differs from pull request most recent head 402faa4. Consider uploading reports for the commit 402faa4 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #657      +/-   ##
==========================================
- Coverage   80.73%   75.47%   -5.26%     
==========================================
  Files          49       60      +11     
  Lines        3415     4404     +989     
==========================================
+ Hits         2757     3324     +567     
- Misses        658     1080     +422     
Flag Coverage Δ
python-3.10 75.47% <ø> (-5.26%) ⬇️
python-3.8 ?
python-3.9 75.50% <ø> (-5.27%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AndresOrtegaGuerrero
Copy link
Member

@superstar54 Nice, I will test it and let you know, I was wondering. If an user has a WorkChain , maybe lets say a bands , but the structure as an input is coming from another WorkChain, and by deleting the first node it deletes automatically all the linked nodes. Is this avoided?

@superstar54
Copy link
Member Author

If an user has a WorkChain , maybe lets say a bands , but the structure as an input is coming from another WorkChain, and by deleting the first node it deletes automatically all the linked nodes. Is this avoided?

Good question. I will look into it and provide more information to the user when deleting nodes.

@AndresOrtegaGuerrero
Copy link
Member

@superstar54 I really like the job list, is easy to search through jobs. I could be nice if there is a choice to add some filters like, filter by state : Finished[0] , or properties. I guess in this page the search app from edan could be useful.

@AndresOrtegaGuerrero
Copy link
Member

@superstar54 I would suggest you add a section explaning that by deleting a node (QeWorkChain) all the associated and link nodes will be eliminated , as well as other calculations that might be linked with this node.

delete.ipynb Outdated
" print(f'Node {pk} deleted.')\n",
"\n",
"# Ask for confirmation\n",
"if input(f'Do you really want to delete node {pk}? [y/N] ').strip().lower() == 'y':\n",
Copy link
Member

@AndresOrtegaGuerrero AndresOrtegaGuerrero Apr 8, 2024

Choose a reason for hiding this comment

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

Also It might be a good idea to display the number nodes associated, so the person know what nodes or workchains are being deleted

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I display the number of nodes to be deleted.

@superstar54
Copy link
Member Author

I really like the job list, is easy to search through jobs. I could be nice if there is a choice to add some filters like, filter by state : Finished[0] , or properties. I guess in this page the search app from edan could be useful.

Thanks, I added basic search filters. We can use @edan-bainglass 's query builder when it's ready in the future.

Screenshot from 2024-04-08 17-04-14

@superstar54
Copy link
Member Author

superstar54 commented Apr 11, 2024

from @cpignedoli and @giovannipizzi

  • Add label/description for search.
  • allow edit the label/description.

Add extras for the properties

@superstar54
Copy link
Member Author

Not delete a job, but hide it.

@superstar54
Copy link
Member Author

Hi @AndresOrtegaGuerrero , I made the changes as you suggested. Here is the screenshot of the page when user deleting a node. Could you please have another look?

Screenshot from 2024-04-24 07-39-26

delete.ipynb Outdated
"source": [
"# AiiDAlab QauntumESPRESSO App\n",
"\n",
"<font color=\"red\"><b>Caution!</b></font> Deleting this job will also remove <b>all associated nodes</b>, including any related calculations and their respective results. This action is <b>irreversible</b>.\n"

Choose a reason for hiding this comment

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

<font color="red"><b>Caution!</b></font> Deleting this job will also remove <b>all associated nodes</b>, including any workflows that share a common input structure, as well as various calculations and their respective results. <b>This action is irreversible.</b>

Copy link
Member Author

@superstar54 superstar54 Apr 24, 2024

Choose a reason for hiding this comment

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

Hi Andres, thanks for the comment.

including any workflows that share a common input structure

It will not delete the workflows that share the input structure. It only deletes its descendants.

  • If QEApp jobs A and B use the same input structure, if A is deleted, B will not.
  • If QEApp job A has an output structure and job B uses it as input, then if A is deleted, B will also be deleted. I added a check here; in this case, the user can not delete job A.

delete.ipynb Outdated Show resolved Hide resolved
@AndresOrtegaGuerrero
Copy link
Member

Hey Xing , it looks good to me , just check the few typos. I was wondering something, is it possible to check if the selected node to deete has other QeAppWorkChain associated ? and display this ? if not we can leave it for a future PR

@superstar54
Copy link
Member Author

Hi @AndresOrtegaGuerrero , thanks for the review and suggestion. I have implemented the changes as you suggested.

I was wondering something, is it possible to check if the selected node to deete has other QeAppWorkChain associated ? and display this ? if not we can leave it for a future PR

Yes, it's possible. I have added a check to see if there is a QEApp job using the outputs of the nodes that need to be deleted. If there is, the deletion will be blocked. Only after the user deletes the related QEApp job can the root one be deleted.

Screenshot from 2024-04-24 12-06-28

Copy link
Member

@AndresOrtegaGuerrero AndresOrtegaGuerrero left a comment

Choose a reason for hiding this comment

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

LGTM!

@superstar54 superstar54 merged commit ae79e3e into aiidalab:main Apr 24, 2024
14 checks passed
@superstar54 superstar54 deleted the feature/add_search_result_page branch April 24, 2024 12:04
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.

Add search page
2 participants