config: Add support for remapping Postgres configs into Gel enums #8275
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have a long-standing rule that configuration parameters should avoid
using a boolean type and use enums instead (though we've not generally
been super diligent about this always). Postgres, on the other hand,
has lots of boolean settings. To solve this, teach the config framework
how to remap backend config values onto arbitrary frontend enums. In
general the below is all that is required:
class EnabledDisabledEnum(enum.StrEnum):
Enabled = "Enabled"
Disabled = "Disabled"
class EnabledDisabledType(
EnumScalarType[EnabledDisabledEnum],
edgeql_type="cfg::TestEnabledDisabledEnum",
):
@classmethod
def get_translation_map(cls) -> Mapping[EnabledDisabledEnum, str]:
return {
EnabledDisabledEnum.Enabled: "true",
EnabledDisabledEnum.Disabled: "false",
}
msullivan
approved these changes
Jan 30, 2025
Comment on lines
+647
to
+676
| def __reduce__(self) -> tuple[ | ||
| Callable[..., EnumScalarType[Any]], | ||
| tuple[ | ||
| Optional[tuple[type, ...] | type], | ||
| E, | ||
| ], | ||
| ]: | ||
| assert type(self).is_fully_resolved(), \ | ||
| f'{type(self)} parameters are not resolved' | ||
|
|
||
| cls: type[EnumScalarType[E]] = self.__class__ | ||
| types: Optional[tuple[type, ...]] = self.orig_args | ||
| if types is None or not cls.is_anon_parametrized(): | ||
| typeargs = None | ||
| else: | ||
| typeargs = types[0] if len(types) == 1 else types | ||
| return (cls.__restore__, (typeargs, self._val)) | ||
|
|
||
| @classmethod | ||
| def __restore__( | ||
| cls, | ||
| typeargs: Optional[tuple[type, ...] | type], | ||
| val: E, | ||
| ) -> Self: | ||
| if typeargs is None or cls.is_anon_parametrized(): | ||
| obj = cls(val) | ||
| else: | ||
| obj = cls[typeargs](val) # type: ignore[index] | ||
|
|
||
| return obj |
There was a problem hiding this comment.
What do we need all this special handling/parametric type stuff for? How are we using the type param?
(Probably add a comment explaining)
There was a problem hiding this comment.
Parametric basically gives us runtime access to the generic type arg and lets us avoid copy-pasting the constructors. Given a bunch of enums are added #8276 (and possibly more settings later) I figured it's worth complicating the base type to make implementations essentially glorified mappings.
edb/pgsql/metaschema.py
Outdated
| class PostgresJsonConfigValueToGelConfigValueFunction( | ||
| trampoline.VersionedFunction, | ||
| ): | ||
| """Convert a Postgres config value to Gel config value. |
There was a problem hiding this comment.
I think my inclination is to keep calling it "edgedb" as a "codename" in internal code like this, but I don't feel strongly.
Just makes it a little easier to be consistent in function names and stuff, though obviously it is kind of fucked no matter what.
There was a problem hiding this comment.
We also have the backend (postgres) and frontend (edgedb-server) used around (which I think I prefer)
elprans
added a commit
that referenced
this pull request
Jan 31, 2025
…estmode When adding remapping machinery in #8275 I forgot that `_testmode` config gets grafted onto bootstrap cache shipped in production builds. Rectify that.
msullivan
pushed a commit
that referenced
this pull request
Feb 4, 2025
) We have a long-standing rule that configuration parameters should avoid using a boolean type and use enums instead (though we've not generally been super diligent about this always). Postgres, on the other hand, has lots of boolean settings. To solve this, teach the config framework how to remap backend config values onto arbitrary frontend enums. In general the below is all that is required: class EnabledDisabledEnum(enum.StrEnum): Enabled = "Enabled" Disabled = "Disabled" class EnabledDisabledType( EnumScalarType[EnabledDisabledEnum], edgeql_type="cfg::TestEnabledDisabledEnum", ): @classmethod def get_translation_map(cls) -> Mapping[EnabledDisabledEnum, str]: return { EnabledDisabledEnum.Enabled: "true", EnabledDisabledEnum.Disabled: "false", } BACKPORT NOTES: The patches won't work until the next two commits are applied. Fixing patches wound up being downstream of this PR in annoying way.
msullivan
added
backported-6.x
msullivan
pushed a commit
that referenced
this pull request
Feb 4, 2025
) We have a long-standing rule that configuration parameters should avoid using a boolean type and use enums instead (though we've not generally been super diligent about this always). Postgres, on the other hand, has lots of boolean settings. To solve this, teach the config framework how to remap backend config values onto arbitrary frontend enums. In general the below is all that is required: class EnabledDisabledEnum(enum.StrEnum): Enabled = "Enabled" Disabled = "Disabled" class EnabledDisabledType( EnumScalarType[EnabledDisabledEnum], edgeql_type="cfg::TestEnabledDisabledEnum", ): @classmethod def get_translation_map(cls) -> Mapping[EnabledDisabledEnum, str]: return { EnabledDisabledEnum.Enabled: "true", EnabledDisabledEnum.Disabled: "false", } BACKPORT NOTES: The patches won't work until the next two commits are applied. Fixing patches wound up being downstream of this PR in annoying way.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have a long-standing rule that configuration parameters should avoid
using a boolean type and use enums instead (though we've not generally
been super diligent about this always). Postgres, on the other hand,
has lots of boolean settings. To solve this, teach the config framework
how to remap backend config values onto arbitrary frontend enums. In
general the below is all that is required: