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

Typing of deployment's payload field #178

Closed
msw-kialo opened this issue Jan 8, 2025 · 6 comments · Fixed by #181
Closed

Typing of deployment's payload field #178

msw-kialo opened this issue Jan 8, 2025 · 6 comments · Fixed by #181
Labels
bug Something isn't working schema schema related WebHook

Comments

@msw-kialo
Copy link

msw-kialo commented Jan 8, 2025

GitHub deployments API includes a payload field. That can be JSON payload (either as string or directly embedded).

It is currently documented as:

payload:
  oneOf:
    - type: object
      additionalProperties: true
    - type: string
      description: JSON payload with extra information about the deployment.
      default: ''

it is currently typed as

class ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type(TypedDict):
    """ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0"""


class WebhookDeploymentCreatedPropDeploymentPropPayloadOneof0(GitHubModel):
    """WebhookDeploymentCreatedPropDeploymentPropPayloadOneof0"""

Both represent the type: object aspect, but ignore additionalProperties: true. This makes the field useless (as they only allow to encode an empty json object).

For pydantic extra=allow is needed:

class WebhookDeploymentCreatedPropDeploymentPropPayloadOneof0(GitHubModel):
    """WebhookDeploymentCreatedPropDeploymentPropPayloadOneof0"""

	model_config = {"extra": "allow"}

I am not so sure about ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type. I think, it needs to be encoded as a normal dict[str, ...] as TypedDicts have currently no way to allow for extra keys.

(I can try to create a PR but I am not familiar with the code base at the moment).

@yanyongyu yanyongyu added bug Something isn't working schema schema related WebHook labels Jan 8, 2025
@yanyongyu
Copy link
Owner

Actually, there is a ExtraGitHubModel can be inheritted. It allows extra fields. additional properties option is partially supported. I need to check the codegen to locate the root reason of this issue. Maybe the parser or the template cause this.

@yanyongyu
Copy link
Owner

It seems the schema is

image

without additionalProperties. Where did you get the schema from?

We can make a schema patch to fix this.

@msw-kialo
Copy link
Author

I looked at https://github.com/github/rest-api-description/blob/main/descriptions/api.github.com/api.github.com.2022-11-28.yaml.

However, I only checked paths./repos/{owner}/{repo}/deployments.post.requestBody.content.application/json.schema.properties.payload (line 28964) and components.schemas.deployment.properties.payload (line 80860).
I see now that components.schemas.webhook-deployment-created.properties.deployment.properties.payload (line 106974) and components.schemas.webhook-deployment-status-created.properties.deployment.properties.payload (line 110250) indeed does not specify additionalProperties (even so they are returned if the deployment is created with them).
I checked the online documentation https://docs.github.com/en/webhooks/webhook-events-and-payloads#deployment (but they only specific object or string).

I reported this upstream: github/rest-api-description#4421

@msw-kialo
Copy link
Author

Thank you for correcting the webhook model. 🙏

However, using a plain TypedDict for the API calls isn't sufficient as it does not allow additional fields:

from githubkit.versions.latest.types import ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type

empty : ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type = {}

some_field_bool : ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type = {"downtime": False}
some_field_str : ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type = {"downtime": "false"}
> pyright
payload_test.py
  payload_test.py:5:77 - error: Type "dict[str, bool]" is not assignable to declared type "ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type"
    "downtime" is an undefined item in type "ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type" (reportAssignmentType)
  payload_test.py:6:76 - error: Type "dict[str, str]" is not assignable to declared type "ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type"
    "downtime" is an undefined item in type "ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type" (reportAssignmentType)
2 errors, 0 warnings, 0 informations 
> mypy
payload_test.py:5: error: Extra key "downtime" for TypedDict "ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type"  [typeddict-unknown-key]
payload_test.py:6: error: Extra key "downtime" for TypedDict "ReposOwnerRepoDeploymentsPostBodyPropPayloadOneof0Type"  [typeddict-unknown-key]

I think for the time being additionalProperties: true must allow a plain dict?

@yanyongyu
Copy link
Owner

yanyongyu commented Jan 13, 2025

object without any field and allow extra properties can be a plain dict. Since this is not relative to the webhook schema, could you please create a separate issue to track this?

@msw-kialo
Copy link
Author

Of cause: #182

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working schema schema related WebHook
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants