feat: add secret parameter type for pipeline parameters(HEXA-1457 )#373
feat: add secret parameter type for pipeline parameters(HEXA-1457 )#373DimitriKwihangana wants to merge 3 commits intomainfrom
Conversation
DimitriKwihangana
changed the title
DimitriKwihangana
changed the title
| int = "int" | ||
| postgresql = "postgresql" | ||
| s3 = "s3" | ||
| secret = "secret" |
There was a problem hiding this comment.
To modify the files under graphql/graphql_client/ you need to follow https://github.com/BLSQ/openhexa-sdk-python/blob/0f795c065543956e8487d3b1a126e65a3a09ea1a/README.md#codegen-from-the-graphql-schema
Those files are auto generated based on schema.generated.graphql
There was a problem hiding this comment.
I know I'm off today :) but felt to address the comments. It slipped my mind that files under graphql/graphql_client/ are auto-generated. I've added secret to schema.generated.graphql and re-ran ariadne-codegen to regenerate enums.py instead of editing it manually.
| if isinstance(type, ParameterType): | ||
| self.type = type | ||
| else: | ||
| self.type = TYPES_BY_PYTHON_TYPE[type.__name__]() |
There was a problem hiding this comment.
Why is this change required ? Just curious
There was a problem hiding this comment.
runtime.py passes a ParameterType instance directly to Parameter.init, so this check was needed to handle that path and it worked. let me know if it should be this way. not so sure if it is the perfect way.
There was a problem hiding this comment.
It seems a smell that something is wrong, because conceptually we just want to add a new type to but we modify the generic logic for all types
I think #374 would solve it (not particularly polished nor tested)
| param_kwargs = {k: v["value"] for k, v in parameter_args.items()} | ||
|
|
||
| parameter = Parameter(type=type_class.expected_type, **param_kwargs) | ||
| parameter = Parameter(type=type_class, **param_kwargs) |
There was a problem hiding this comment.
Why is this change required ? Just curious
There was a problem hiding this comment.
Before I was having an error of having the secret reaching in the database treated as "str" I found that without this fix, SecretType().expected_type resolves to str, meaning Secret parameters would silently serialize as "type": "str" and never reach the frontend as masked.
There was a problem hiding this comment.
Ok, I think the type logic smells wrong for the Secret and this seems to fix the symptom but not the underlying issue
What do you think of this #374 ? (not particularly polished nor tested)
2 participants
Adds a new Secret parameter type so pipeline authors can mark sensitive values (e.g. API tokens) to be masked in the app instead of displayed in a string.
Changes
Please list / describe the changes in the codebase for the reviewer(s).
Implemented the type on the application too feat: add secret parameter type for pipeline parameters openhexa-app#1719