PASTA-ELN / pasta-eln

PASTA-ELN with new frontend
https://pasta-eln.github.io/pasta-eln/
Other
7 stars 2 forks source link

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

Closed jmurugan-fzj closed 1 week ago

jmurugan-fzj commented 2 weeks ago
jmurugan-fzj commented 2 weeks ago

@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!

jmurugan-fzj commented 2 weeks ago

@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 commented 2 weeks ago

@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 commented 2 weeks ago

@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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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.

jmurugan-fzj commented 2 weeks ago

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!