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

InfluxDB 2.0: The "View" doesn't have defined all types of "ViewProperties" [area/api] #12972

Closed
bednar opened this issue Mar 28, 2019 · 5 comments · Fixed by #13899
Closed

Comments

@bednar
Copy link
Contributor

bednar commented Mar 28, 2019

The View is defined as

View:
      properties:
        links:
          type: object
          readOnly: true
          properties:
            self:
              type: string
        id:
          readOnly: true
          type: string
        name:
          type: string
        properties:
          oneOf:
            - $ref: "#/components/schemas/V1ViewProperties"
            - $ref: "#/components/schemas/EmptyViewProperties"
            - $ref: "#/components/schemas/LogViewProperties"

but there are missing MarkdownViewProperties, TableViewProperties,XYViewProperties, ... that are defined in sources:

// LinePlusSingleStatProperties represents options for line plus single stat view in Chronograf
type LinePlusSingleStatProperties struct {
	Queries           []DashboardQuery `json:"queries"`
	Axes              map[string]Axis  `json:"axes"`
	Type              string           `json:"type"`
	Legend            Legend           `json:"legend"`
	ViewColors        []ViewColor      `json:"colors"`
	Prefix            string           `json:"prefix"`
	Suffix            string           `json:"suffix"`
	DecimalPlaces     DecimalPlaces    `json:"decimalPlaces"`
	Note              string           `json:"note"`
	ShowNoteWhenEmpty bool             `json:"showNoteWhenEmpty"`
}

// XYViewProperties represents options for line, bar, step, or stacked view in Chronograf
type XYViewProperties struct {
	Queries           []DashboardQuery `json:"queries"`
	Axes              map[string]Axis  `json:"axes"`
	Type              string           `json:"type"`
	Legend            Legend           `json:"legend"`
	Geom              string           `json:"geom"` // Either "line", "step", "stacked", or "bar"
	ViewColors        []ViewColor      `json:"colors"`
	Note              string           `json:"note"`
	ShowNoteWhenEmpty bool             `json:"showNoteWhenEmpty"`
}

// SingleStatViewProperties represents options for single stat view in Chronograf
type SingleStatViewProperties struct {
	Type              string           `json:"type"`
	Queries           []DashboardQuery `json:"queries"`
	Prefix            string           `json:"prefix"`
	Suffix            string           `json:"suffix"`
	ViewColors        []ViewColor      `json:"colors"`
	DecimalPlaces     DecimalPlaces    `json:"decimalPlaces"`
	Note              string           `json:"note"`
	ShowNoteWhenEmpty bool             `json:"showNoteWhenEmpty"`
}

// HistogramViewProperties represents options for histogram view in Chronograf
type HistogramViewProperties struct {
	Type              string           `json:"type"`
	Queries           []DashboardQuery `json:"queries"`
	ViewColors        []ViewColor      `json:"colors"`
	XColumn           string           `json:"xColumn"`
	FillColumns       []string         `json:"fillColumns"`
	XDomain           []float64        `json:"xDomain,omitEmpty"`
	XAxisLabel        string           `json:"xAxisLabel"`
	Position          string           `json:"position"`
	BinCount          int              `json:"binCount"`
	Note              string           `json:"note"`
	ShowNoteWhenEmpty bool             `json:"showNoteWhenEmpty"`
}

// GaugeViewProperties represents options for gauge view in Chronograf
type GaugeViewProperties struct {
	Type              string           `json:"type"`
	Queries           []DashboardQuery `json:"queries"`
	Prefix            string           `json:"prefix"`
	Suffix            string           `json:"suffix"`
	ViewColors        []ViewColor      `json:"colors"`
	DecimalPlaces     DecimalPlaces    `json:"decimalPlaces"`
	Note              string           `json:"note"`
	ShowNoteWhenEmpty bool             `json:"showNoteWhenEmpty"`
}

// TableViewProperties represents options for table view in Chronograf
type TableViewProperties struct {
	Type              string           `json:"type"`
	Queries           []DashboardQuery `json:"queries"`
	ViewColors        []ViewColor      `json:"colors"`
	TableOptions      TableOptions     `json:"tableOptions"`
	FieldOptions      []RenamableField `json:"fieldOptions"`
	TimeFormat        string           `json:"timeFormat"`
	DecimalPlaces     DecimalPlaces    `json:"decimalPlaces"`
	Note              string           `json:"note"`
	ShowNoteWhenEmpty bool             `json:"showNoteWhenEmpty"`
}

type MarkdownViewProperties struct {
	Type string `json:"type"`
	Note string `json:"note"`
}

For example the MarkdownViewProperties should be defined as:

MarkdownViewProperties:
      type: object
      required:
      - note
      - shape
      - type
      properties:
        type:
          type: string
          enum: ["markdown"]
        shape:
          type: string
          enum: ["chronograf-v2"]
        note:
          type: string
@imogenkinsman
Copy link
Contributor

I think V1ViewProperties is meant to encompass multiple types:

        graphType:
          description: The viewport for a view's graph/visualization
          type: string
          enum:
            - single-stat
            - line
            - line-plus-single-stat
            - line-stacked
            - line-stepplot
            - bar
            - gauge
            - table

But I'm not sure if the unique fields for several of these types are correctly reflected in our swagger. I'm going through each and figuring out what's missing, V1ViewProperties will probably need to be split into multiple view properties.

@bednar
Copy link
Contributor Author

bednar commented Apr 3, 2019

I agree. The best approach will be split V1ViewProperties into multiple properties:

View:
      properties:
        links:
          type: object
          readOnly: true
          properties:
            self:
              type: string
        id:
          readOnly: true
          type: string
        name:
          type: string
        properties:
          oneOf:
            - $ref: "#/components/schemas/LinePlusSingleStatProperties"
            - $ref: "#/components/schemas/XYViewProperties"
            - $ref: "#/components/schemas/SingleStatViewProperties"
            - $ref: "#/components/schemas/HistogramViewProperties"
            - $ref: "#/components/schemas/GaugeViewProperties"
            - $ref: "#/components/schemas/TableViewProperties"
            - $ref: "#/components/schemas/MarkdownViewProperties"
            - $ref: "#/components/schemas/LogViewProperties"
            - $ref: "#/components/schemas/EmptyViewProperties"

@bednar
Copy link
Contributor Author

bednar commented May 27, 2019

The changed "swagger" doesn't respect implementation. The unmarshalling of view properties depends on shape property (see sources):

func UnmarshalViewPropertiesJSON(b []byte) (ViewProperties, error) {
	var v struct {
		B json.RawMessage `json:"properties"`
	}

	if err := json.Unmarshal(b, &v); err != nil {
		return nil, err
	}

	if len(v.B) == 0 {
		// Then there wasn't any visualization field, so there's no need unmarshal it
		return EmptyViewProperties{}, nil
	}

	var t struct {
		Shape string `json:"shape"`
		Type  string `json:"type"`
	}

	if err := json.Unmarshal(v.B, &t); err != nil {
		return nil, err
	}

	var vis ViewProperties
	switch t.Shape {
	case "chronograf-v2":
		switch t.Type {
		case "xy":
			var xyv XYViewProperties
			if err := json.Unmarshal(v.B, &xyv); err != nil {
				return nil, err
			}
			vis = xyv
		case "single-stat":
			var ssv SingleStatViewProperties
			if err := json.Unmarshal(v.B, &ssv); err != nil {
				return nil, err
			}
			vis = ssv
		case "gauge":
			var gv GaugeViewProperties
			if err := json.Unmarshal(v.B, &gv); err != nil {
				return nil, err
			}
...

but LinePlusSingleStatProperties, XYViewProperties, SingleStatViewProperties, ... doesn't have shape property.


The Views schema is useless and was removed in #13152.

@bednar bednar reopened this May 27, 2019
@stale
Copy link

stale bot commented Oct 31, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 31, 2019
@stale
Copy link

stale bot commented Nov 7, 2019

This issue has been automatically closed because it has not had recent activity. Please reopen if this issue is still important to you. Thank you for your contributions.

@stale stale bot closed this as completed Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants