Mikubill / sd-webui-controlnet

WebUI extension for ControlNet
GNU General Public License v3.0
16.89k stars 1.94k forks source link

[Bug]: Enums through API are broken due to validation #2898

Closed stassius closed 4 months ago

stassius commented 4 months ago

Is there an existing issue for this?

What happened?

At the moment enums control_mode and resize_mode require to pass the full strings from UI (Like "My prompt is more important") through the API instead of previously accepted numbers. I think it's an extremely bad and error-prone architecture. We should be able to use, say, keys, like "BALANCED", "PROMPT", "CONTROL" or numbers, as we used to.

Steps to reproduce the problem

Try to use API.

What should have happened?

It should allow to use at least numbers or enum keys instead of values.

Commit where the problem happens

webui: controlnet:

What browsers do you use to access the UI ?

No response

Command Line Arguments

-

List of enabled extensions

-

Console logs

pydantic.error_wrappers.ValidationError: 2 validation errors for ControlNetUnit
    resize_mode
       (type=assertion_error)
    control_mode
      value is not a valid enumeration member; permitted: 'Balanced', 'My prompt is more important', 'ControlNet is more important' (type=type_error.enum; enum_values=[<ControlMode.BALANCED: 'Balanced'>, <ControlMode.PROMPT: 'My prompt is more important'>, <ControlMode.CONTROL: 'ControlNet is more important'>])

Additional information

No response

huchenlei commented 4 months ago

If you want to change validation of a specific enum value you can submit a PR to add custom validation on that specific enum field. See https://github.com/Mikubill/sd-webui-controlnet/pull/2861

But for future enums, we are not going to offer the number for full qualified alias option.

huchenlei commented 4 months ago

The breaking changes are stated in https://github.com/Mikubill/sd-webui-controlnet/pull/2847

stassius commented 4 months ago

Soo aliases like "My prompt is more important" and "ControlNet is more important" are something you actually planned to use for real and not by mistake? Is this the great improvement of Enum handling you mentioned?

Are there any guarantees that they will not change one day, rendering all the third party application broken?

huchenlei commented 4 months ago

Soo aliases like "My prompt is more important" and "ControlNet is more important" are something you actually planned to use for real and not by mistake? Is this the great improvement of Enum handling you mentioned?

Are there any guarantees that they will not change one day, rendering all the third party application broken?

If enum names are going to change in the future, we will add custom validation logic to make sure old enum names can still be recognized. The same way as in https://github.com/Mikubill/sd-webui-controlnet/pull/2861

huchenlei commented 4 months ago

Soo aliases like "My prompt is more important" and "ControlNet is more important" are something you actually planned to use for real and not by mistake? Is this the great improvement of Enum handling you mentioned?

Are there any guarantees that they will not change one day, rendering all the third party application broken?

The improvement on enum handling is that we are sure each field contains a enum type instance, not some random type that needs conversion on reference.