Skip to content

Conversation

@filvecchiato
Copy link
Contributor

This PR aims to introduce a new method to inject the required/available controllers for a chain in express by using the pallets defined in the metadata of such chain. It also allows to add a config file in chains_config and then use that by setting SAS_EXPRESS_CONTROLLERS_CONFIG to true.

Each controller now requires two new static variables: controllerName and requiredPallets

ControllerName is the name of the controller to be used in the configs and requiredPallets are the pallets required in the metadata to have the controller injected in the server.

@filvecchiato filvecchiato requested a review from a team as a code owner February 13, 2025 16:17
@filvecchiato filvecchiato changed the title FEAT: inject controllers using metadata's definition of pallets feat: inject controllers using metadata's definition of pallets Feb 13, 2025
@TarikGul
Copy link
Member

Question: Don't we no longer need the chains-config controllers key anymore after this PR?

@filvecchiato
Copy link
Contributor Author

Question: Don't we no longer need the chains-config controllers key anymore after this PR?

It depends if the user decides to inject the controllers or use configurations using the SAS_EXPRESS_CONTROLLERS_CONFIG flag. I'm also using it for the tests but happy to move it outside/on a separate config file

Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

LGTM!

@filvecchiato filvecchiato force-pushed the filippo_dynamic_controllers branch from 9326d44 to afa027c Compare February 22, 2025 15:19
@filvecchiato filvecchiato requested a review from Imod7 March 2, 2025 13:31
Copy link
Contributor

@Imod7 Imod7 left a comment

Choose a reason for hiding this comment

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

Looks great! 💯 This is such a great feature so kudos for the initiative and the excellent suggestion. Also, the implementation was super fast!

@filvecchiato filvecchiato merged commit 2aa6e6a into master Mar 3, 2025
12 checks passed
@filvecchiato filvecchiato deleted the filippo_dynamic_controllers branch March 3, 2025 13:08
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.

Improvement: Switch chain based configuration of controllers to pallet based configuration

5 participants