Skip to content
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

[Enhancement]: schema inference does not work well for nested layout when there's dynamic fields defined #1707

Open
Yufeireal opened this issue Oct 21, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request evaluator

Comments

@Yufeireal
Copy link

General Question

Hi team, I've posted a question in our kcl slack channel: https://cloud-native.slack.com/archives/C05TC96NWN8/p1729279836481899
File an issue here in case it's a bug.

When defining schemas like this

schema ContainerInputs:
    maxReplicas?: int = 3

schema DeploymentInputs:
    [...str]: ContainerInputs

schema DeploymentCollectionInputs:
    [...str]: DeploymentInputs

    
    
schema NewDeploymentInputs(DeploymentInputs):
    [...str]: NewContainerInputs

schema NewContainerInputs(ContainerInputs):
    minReplicas?: int = 2

    

schema NewDeploymentCollectionInputs(DeploymentCollectionInputs):
    [...str]: NewDeploymentInputs

schema Inputs:
    vdeployments: DeploymentCollectionInputs

schema NewInputs(Inputs):
    vdeployments: NewDeploymentCollectionInputs

a = Inputs {
    vdeployments: {
        deploy_a: {
            container_a: {
                maxReplicas: 4
            }
        }
    }
}
# When trying to use inheritance, got `expect ContainerInputs, got dict`. Expect to get {minReplicas: 2, maxReplicas: 3}?
b = NewInputs {
    vdeployments: {
        deploy_b: {
            container_b: {
              minReplicas: 2
            }
        }
    }
}

Playground link

I got an error about expect ContainerInputs, but got a dict. However, if I change the initialization to be:

b = NewInputs {
    vdeployments: {
        deploy_b: {
            container_b: NewContainerInputs {
               minReplicas: 3
            }
        }
    }
}

It works.

Not sure if this is expected when base schema has dynamic fields and we does not support override those dynamic fields right now. Need some help here, thanks!

@Peefy Peefy added bug Something isn't working evaluator labels Oct 22, 2024
@Peefy Peefy added this to the v0.11.0 Release milestone Oct 22, 2024
@Peefy Peefy added enhancement New feature or request and removed bug Something isn't working labels Oct 22, 2024
@Peefy
Copy link
Contributor

Peefy commented Oct 22, 2024

Yes, this is due to the imperfect running mechanism of the KCL evaluator, although the IDE can recognize it correctly. We will improve it in future versions, and now you need to explicitly specify the type.

b = NewInputs {
    vdeployments: NewDeploymentCollectionInputs {
        deploy_b: {
            container_b: {
                minReplicas: 2
            }
        }
    }
}

or

b = NewInputs {
    vdeployments: {
        deploy_b: {
            container_b: NewContainerInputs {
                minReplicas: 2
            }
        }
    }
}

@Yufeireal
Copy link
Author

Got it!! Thanks for taking a look.

@Peefy Peefy changed the title [Potential Bug, TBD]: schema inference does not work well for nested layout when there's dynamic fields defined [Enhancement]: schema inference does not work well for nested layout when there's dynamic fields defined Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request evaluator
Projects
None yet
Development

No branches or pull requests

3 participants