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

[elm] Process additionalProperties in parent alias when using composition (allOf) #1465

Closed
mxinden opened this issue Nov 16, 2018 · 4 comments

Comments

@mxinden
Copy link
Contributor

mxinden commented Nov 16, 2018

Description

This is a follow up to #1140 and #1262.

When I use the allOf keyword to compose GettableAlert based on Alert, the Annotation is not defined as a Dict String String but as a LabelSet. But as discussed in #1140 LabelSet is not properly generated.

openapi-generator version

3.3.4-SNAPSHOT

OpenAPI declaration file content or url
---

swagger: '2.0'

info:
  version: 0.0.1
  title: Alertmanager API
  description: API of the Prometheus Alertmanager (https://github.com/prometheus/alertmanager)

paths:
  /alert:
    get:
      operationId: getAlert
      description: Get an alert
      parameters:
      responses:
        '200':
          description: Get alerts response
          schema:
              '$ref': '#/definitions/alert'
definitions:
  alert:
    type: object
    properties:
      labels:
        $ref: '#/definitions/labelSet'
    required:
      - labels
  gettableAlert:
    allOf:
      - type: object
        properties:
          annotations:
            $ref: '#/definitions/labelSet'
        required:
          - annotations
      - $ref: '#/definitions/alert'
  labelSet:
    type: object
    additionalProperties:
      type: string
Command line used for generation

docker run --user=$(id -u ${USER}):$(id -g ${USER}) --rm -v ${PWD}:/local openapitools/openapi-generator-cli:latest generate -i /local/openapi.yaml -g elm -o /local/example

Steps to reproduce

Run above docker command with the above written OpenAPI specification. This results in:

{-
   Alertmanager API
   API of the Prometheus Alertmanager (https://github.com/prometheus/alertmanager)

   OpenAPI spec version: 0.0.1

   NOTE: This file is auto generated by the openapi-generator.
   https://github.com/openapitools/openapi-generator.git
   Do not edit this file manually.
-}


module Data.GettableAlert exposing (GettableAlert, decoder, encoder)

import Data.LabelSet as LabelSet exposing (LabelSet)
import Dict exposing (Dict)
import Json.Decode as Decode exposing (Decoder)
import Json.Decode.Pipeline exposing (optional, required)
import Json.Encode as Encode


type alias GettableAlert =
    { labels : Dict String String
    -- I would expect `Dict String Strng` here as well.
    , annotations : LabelSet
    }


decoder : Decoder GettableAlert
decoder =
    Decode.succeed GettableAlert
        |> required "labels" (Decode.dict Decode.string)
        |> required "annotations" LabelSet.decoder



encoder : GettableAlert -> Encode.Value
encoder model =
    Encode.object
        [ ( "labels", (Encode.dict identity Encode.string) model.labels )
        , ( "annotations", LabelSet.encoder model.annotations )

        ]
Related issues/PRs

#1140 and #1262

@mxinden
Copy link
Contributor Author

mxinden commented Nov 21, 2018

@wing328 @trenneman would you mind taking a look at this?

@eriktim
Copy link
Contributor

eriktim commented Nov 21, 2018

@wing328 Am I correct to say this is a general issue? I'd actually prefer to have the alias LabelSet here which makes it much easier to read IMHO.

@wing328
Copy link
Member

wing328 commented Nov 24, 2018

@trenneman yup, it should be fixed by #1296

mxinden added a commit to mxinden/alertmanager that referenced this issue Nov 28, 2018
With issue 1465 on openapi-generator [1] being fixed, we can not extract
shared properties of the gettable and postable alert definition into a
shared object (`alert`) like we do for silence, gettable silence and
postable silence.

In addition this patch does the following changes to the UI:

- Use `List GettableAlert` instead of plural type definition like
`GettableAlerts` because the plural definitions are not generated.

- Fix openapi-generator-cli docker image to specific hash.

[1] OpenAPITools/openapi-generator#1465

Signed-off-by: Max Leonard Inden <IndenML@gmail.com>
mxinden added a commit to mxinden/alertmanager that referenced this issue Nov 28, 2018
With issue 1465 on openapi-generator [1] being fixed, we can not extract
shared properties of the gettable and postable alert definition into a
shared object (`alert`) like we do for silence, gettable silence and
postable silence.

In addition this patch does the following changes to the UI:

- Use `List GettableAlert` instead of plural type definition like
`GettableAlerts` because the plural definitions are not generated.

- Fix openapi-generator-cli docker image to specific hash.

[1] OpenAPITools/openapi-generator#1465

Signed-off-by: Max Leonard Inden <IndenML@gmail.com>
@mxinden
Copy link
Contributor Author

mxinden commented Nov 28, 2018

This is fixed with #1296! Thanks a lot for the help @wing328 and @trenneman.

Fix further downstream in Alertmanager: prometheus/alertmanager#1640.

@mxinden mxinden closed this as completed Nov 28, 2018
mxinden added a commit to mxinden/alertmanager that referenced this issue Nov 28, 2018
With issue 1465 on openapi-generator [1] being fixed, we can not extract
shared properties of the gettable and postable alert definition into a
shared object (`alert`) like we do for silence, gettable silence and
postable silence.

In addition this patch does the following changes to the UI:

- Use `List GettableAlert` instead of plural type definition like
`GettableAlerts` because the plural definitions are not generated.

- Fix openapi-generator-cli docker image to specific hash.

[1] OpenAPITools/openapi-generator#1465

Signed-off-by: Max Leonard Inden <IndenML@gmail.com>
@wing328 wing328 added this to the 3.3.4 milestone Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants