Skip to content

Conversation

@clarktsiory
Copy link
Contributor

@clarktsiory clarktsiory commented Dec 19, 2025

https://issues.rudder.io/issues/28055

This PR removes the lift-json import from SettingsApi.scala.
It :

  • adds API tests to endpoints that were missing one
  • replaces the toJson and parseJson method of each setting by an idiomatic "JsonCodec instance" in zio-json
    ➡️ the main change is about adding given JsonCodec[...] in place of lift-json. In some places, we need to use the JSON AST types since the settings are implemented in a generic way

Along the way :

EDIT: there is another change, to avoid referencing settings key as hardcoded String, it's possible to extract settings implementation (using ZLayer) and make each setting be an enum

@clarktsiory
Copy link
Contributor Author

Commit modified

@clarktsiory clarktsiory force-pushed the arch_28055/migrate_settingsapi_to_zio_json branch from 714ffc1 to be06578 Compare December 19, 2025 17:44
@clarktsiory clarktsiory force-pushed the arch_28055/migrate_settingsapi_to_zio_json branch from be06578 to ee5dabf Compare December 29, 2025 15:26
@clarktsiory
Copy link
Contributor Author

Commit modified

@clarktsiory clarktsiory force-pushed the arch_28055/migrate_settingsapi_to_zio_json branch from ee5dabf to 9783872 Compare December 29, 2025 16:20
@clarktsiory
Copy link
Contributor Author

Commit modified

@clarktsiory clarktsiory requested a review from P4uline December 29, 2025 16:33
@clarktsiory clarktsiory marked this pull request as ready for review December 29, 2025 16:33
@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

2 similar comments
@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

Copy link
Contributor

@P4uline P4uline left a comment

Choose a reason for hiding this comment

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

LGTM, just squash the commits and I think it's ready to be merged

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/6818
-- Your faithful QA
Kant merge: "Science is organized knowledge. Wisdom is organized life."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/112085/console)

@clarktsiory
Copy link
Contributor Author

OK, squash merging this PR

@clarktsiory clarktsiory force-pushed the arch_28055/migrate_settingsapi_to_zio_json branch from 7ead784 to f0b9a5e Compare January 5, 2026 16:45
@clarktsiory clarktsiory merged commit f0b9a5e into Normation:branches/rudder/9.1 Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants