-
Notifications
You must be signed in to change notification settings - Fork 0
Updated solver schemas #74
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
base: main
Are you sure you want to change the base?
Conversation
| "required": true, | ||
| "default": "No" | ||
| }, | ||
| "activeNetworks": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would we want the activeNetworks and hasActiveNetworks fields if we have the isActive field on the network itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, thinking about this some more I think it makes sense to aggregate these on the solver schema to make it easier to find. Will we use this to show the network names or their addresses? I can see both being useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently I made it contain the network names
bram-vdberg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the schema changes make sense, I'm not sure about adding automation to the CMS though.
First, it disperses complexity into two areas. We would have some automation take place in Dagster and some in the CMS. This means that in the future updates would require changes in two different repo's. One of the main reasons we set up Dagster was to reduce this and keep all data related automation in one place.
Second, other teams depend on the CMS as well. Adding changes to its function might have unintended consequences. Since this could affect other teams I think it's better to be conservative with regards to changes to the CMS.
I would prefer if we only change the schemas and then make updates to the CMS through Dagster using the CMS API.
To be conservative I disabled the lifecycles for now and will revisit it after the first wave of changes (schema + dagster logo check). |
bram-vdberg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we update the bonding pool so that we see the name of the bonding pool here? I think the bonding pool -> solver_bonding pool -> solver setup is still a bit counterintuitive, although I'm not sure right away what would be better.

It would make the vouched_by field easier to understand as well if it said something like 'CoW Bonding pool'.

This pr updates the solver tables schema and adds lifecycle functions that automatically update certain fields based on data in cms. The fields affected by lifecycle are fields that don't require extra data outside of cms and can be handled without human involvement like the logo field that will be handled by a dagster asset.