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

fix(dataverse): integrated changes for handling multiple project groups #304

Conversation

jmurugan-fzj
Copy link
Contributor

@jmurugan-fzj jmurugan-fzj commented Jun 19, 2024

  • Refactored base_database_api.py to take host, port, user and pass as initialization parameters. Removed the db_name property and passed as a parameter for the methods.
  • Adapted database_api.py to keep track of two databases:
    (1) dataverse: for storing all the documents related to dataverse
    (2) default_project_group_db_name that correspond to the default project group database for populating the PASTA projects in UI. Adapted the API calls to use the database names appropriately.
  • Also modified initialize_database to create the necessary documents in both the databases.
  • Make sure that the initialize_database is invoked within the init methods of config_dialog.py and main_dialog.py modules.
  • Removed unwanted initialize_database call from config_dialog.py.
  • Modified and added new tests for the changes.
  • Code reformatting.

…s initialization parameters. Removed the db_name property and passed as a parameter for the methods.

- Adapted the tests for the changes done.
…se: for storing all the documents related to dataverse (2) default_project_group_db_name that correspond to the default project group database for populating the PASTA projects in UI. Adapted the API calls to use the database names appropriately.

- Also modified initialize_database to create the necessary documents in both the databases.
- Make sure that the initialize_database is invoked after the creation of project groups and also while switching to new groups.
- Removed unwanted initialize_database call from config_dialog.py.
- Modified and added new tests for the changes.
- Code reformatting.
@jmurugan-fzj jmurugan-fzj self-assigned this Jun 19, 2024
@jmurugan-fzj jmurugan-fzj added the bug Something isn't working label Jun 19, 2024
@jmurugan-fzj
Copy link
Contributor Author

@SteffenBrinckmann This PR includes the changes from the other PR (#284 Sb python3.12) too, hence the file changes is higher, as soon as the other PR is merged the you would be able to see the changes alone for this PR!

…-data-verse-fails-to-support-upload-in-multiple-user-configured-project-groups
@jmurugan-fzj
Copy link
Contributor Author

@SteffenBrinckmann I have merged the changes from the main branch, the changes are now specific to this PR, please go ahead and review the changes, Thanks

@SteffenBrinckmann
Copy link
Contributor

@jmurugan-fzj Not clear why you add code in projectGroup and especially directly before restart: which restarts and hence looses all information. Why do you not just take the defaultProjectGroup from the configuration file .pastaELN.py?

@jmurugan-fzj
Copy link
Contributor Author

jmurugan-fzj commented Jun 24, 2024

@jmurugan-fzj Not clear why you add code in projectGroup and especially directly before restart: which restarts and hence looses all information. Why do you not just take the defaultProjectGroup from the configuration file .pastaELN.py?

@SteffenBrinckmann Because I saw that "closeDialog" method in projectGroup.py is resposible for saving the contents of .pastaELN.py with the "switched" defaultProjectGroup information. Clearly line numbers 133 & 134 dumps the contents of .pastaELN.py. Isn't this understanding correct? Hence when the user creates and switches the project group, the necessary initialization are done immediately after the dump!

Why do you not just take the defaultProjectGroup from the configuration file .pastaELN.py? Yes, that's what is done exactly inside "db_api.initialize_database()" call, just before the restart; the freshly written defaultProjectGroup and credentials are read from .pastaELN.py and all the necessary views are created along with other initializations and are saved within dataverse DB. Isn't it clear?

@SteffenBrinckmann
Copy link
Contributor

Why don't you check whether the database is set up properly upon starting the data verse tool. If not, set it up. That is far simpler and fault tolerant. There are three ways to change the project group, including editing the JSON file. You cannot track all of them.

@jmurugan-fzj
Copy link
Contributor Author

jmurugan-fzj commented Jun 25, 2024

Why don't you check whether the database is set up properly upon starting the data verse tool. If not, set it up. That is far simpler and fault tolerant. There are three ways to change the project group, including editing the JSON file. You cannot track all of them.

@SteffenBrinckmann I could also add it to the init part of the data-verse tool which will be invoked every-time when the user starts the tool, which is not necessary right? This can affect the performance and the loading time of the tool since this includes file/DB operations and so on. Hence wanted to invoke the method: "db_api.initialize_database()" only once when the user changes the defaultProjectGroup! Doesn't it make sense? Or do you suggest any other way to do this or you think this performance problem is acceptable?

Moreover, why would you allow the user to edit the JSON file directly (error prone way!) when you have the nice GUI option available? :) What's the necessity of such modifications? I would never even include such error prone instructions in the user manual!

@SteffenBrinckmann
Copy link
Contributor

The creation of a view costs much less than a second. The upload minutes to longer, I don't think a user will notice the view creation.

…t methods of config_dialog.py and main_dialog.py modules. Removed the same invocations from gui.py & projectGroup.py.

- Adapted the tests for the changes.
@jmurugan-fzj
Copy link
Contributor Author

jmurugan-fzj commented Jun 25, 2024

The creation of a view costs much less than a second. The upload minutes to longer, I don't think a user will notice the view creation.

@SteffenBrinckmann Okay I moved the initialize_database() to the constructors of config_dialog.py and main_dialog.py, could you please re-look?

Please ignore the failing tests, this happened because one of the "terminology lookup service"-website was down during the test execution!

@jmurugan-fzj jmurugan-fzj merged commit 36414b9 into main Jun 26, 2024
10 checks passed
@jmurugan-fzj jmurugan-fzj deleted the bugs/301-data-verse-fails-to-support-upload-in-multiple-user-configured-project-groups branch June 26, 2024 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data-verse fails to support upload in multiple user configured project groups
2 participants