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

POC for moving some configurations from properties to CouchDB #2815

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hoangnt2
Copy link
Contributor

@hoangnt2 hoangnt2 commented Dec 12, 2024

Note*: This PR just is a POC so only 2 properties are implemented. Other properties will be implemented if this PR is accepted

Idea:

  • Move some configuration properties from sw360.properties file to CouchDB. These properties can be changed without breaking the application

  • The configuration properties related to back-end connection (e.g: backend.url) or migration scripts (e.g: enable.flexible.project.release.relationship) will not be moved to CouchDB

  • The configuration properties will be stored in sw360config database as below
    image

  • The configuration properties will be save and synchronize between memory (Java variable) and CouchDB to avoid query to much into CouchDB. When application is started the configuration properties will be loaded into memory and they also will be updated when there is any update request

New endpoints:

1. Get all configuration properties (from both CouchDB and sw360.properties file)

{
    "spdx.document.enabled": "true",   // Moved properties in CouchDB
    "enable.flexible.project.release.relationship": "true" // Remaining properties in sw360.properties
}

2. Update configuration properties in CouchDB (only moved properties are allowed to change)

{
    "spdx.document.enabled": "false"
}
  • Response:
"Configurations are updated successfully"
  • If configuration properties related to back-end connection or migration scripts are requested to update, return 400 error code with message:
"Invalid config: [${properties name} : ${value}]"

e.g:

"Invalid config: [enable.flexible.project.release.relationship : true]"

Demonstrate configuration changes from the GUI
change-configuration-from-GUI

Please take a look at this PR and feel free to add your comment. If you have any question please let me know

…0 configurations

Signed-off-by: hoangnt2 <hoang2.nguyenthai@toshiba.co.jp>
@hoangnt2
Copy link
Contributor Author

@heliocastro @GMishx, please add your comments (I haven't fixed the unit test yet so the CI fails)

@GMishx
Copy link
Member

GMishx commented Dec 12, 2024

🚀 The idea looks great. In the same endpoint, we can list both, configs from properties file and from DB. This also allows quick update of config in running system, as current way requires restart of tomcat server to update the config constants.

Its a go ahead from me 👍

@smrutis1 , what are your thoughts on this?

@smrutis1
Copy link
Contributor

🚀 The idea looks great. In the same endpoint, we can list both, configs from properties file and from DB. This also allows quick update of config in running system, as current way requires restart of tomcat server to update the config constants.

Its a go ahead from me 👍

@smrutis1 , what are your thoughts on this?

The approach is great. Just an thought: As we are planning to remove thrift in the recent future. Would it be possible here to do it without the thrift layer.

@heliocastro
Copy link
Contributor

heliocastro commented Dec 16, 2024

Something that we will need, we need to have unique ids for each entry related to to the menu itself.
To be more direct, i want to restore the menu json that we construct the interface retrieved from server.
Why that ?
Well, imagine that we have a user that i restricted to access only the licenses page, or an base administrator that can only see the users and the sanitise page ?

@hoangnt2
Copy link
Contributor Author

@heliocastro Yes, we can also retrieve the menu from the server. Do we need to allow the ADMIN user to modify the menu from the GUI? This will make the menu more flexible but could also introduce more problems.

@hoangnt2
Copy link
Contributor Author

@smrutis1 Do we have any alternative solutions to Thrift, or should we simply remove it? Currently, I don't know how to implement this without Thrift.

@smrutis1
Copy link
Contributor

@smrutis1 Do we have any alternative solutions to Thrift, or should we simply remove it? Currently, I don't know how to implement this without Thrift.

I was planning to directly call the backend instead of passing through thrift layer. But lets take it as a separate task for analyzing it better and identify any potential pitfalls. We can proceed with this approach for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants