Python SDK: Enum Support (#242)

* Bugfix version stamping

* Required and optional type properties

added ICodeGen.typeProperties to produce the list of properties for
declareType. The default is just type.properties but mypy/python requires
that all required properties are declared before optional properties on
a class so the python generator overrides this to provide the required
properties first followed by the optional.

* Python SDK: Enum support

refactored serialization structure hooks to handle different types of
ForwardRef objects (enum vs model object)

Broke up structure_hook into reserved_kw_structure_hook and forward_ref_structure_hook

fixed bug where "bare" ForwardRef annotations would bomb out on
deserialization - doesn't exist in the wild yet but it probably will
at some point - except that for now we always mark all API response
model fields as optional because of the "fields" argument...

fixed bug where "reserved_keyword_structure_hook" was not being applied
to the top level Model type.

reversed .python-version list so 3.8.2 is default
added tox-pyenv because tox wasn't finding 3.6 on my box

Co-authored-by: Joel Dodge <joeldodge@google.com>

packages/sdk-codegen-scripts/src/convert.ts

@@ -213,7 +213,7 @@ export const convertSpec = (
     // patch to fix up small errors in source definition (not required, just to ensure smooth process)
     // indent no spaces
     // output to openApiFilename
-    run('swagger2openapi', [
+    run('yarn swagger2openapi', [
       swaggerFilename,
       '-p',
       '-i',

packages/sdk-codegen-scripts/src/fetchSpec.spec.ts

@@ -60,6 +60,11 @@ describe('fetch operations', () => {
     expect(actual).not.toEqual('')
   })
 
+  it('gets lookerVersion with supplied versions', async () => {
+    const actual = await fetchLookerVersion(props, {looker_release_version: '7.10.0'})
+    expect(actual).toEqual('7.10')
+  })
+
   it('gets version info', async () => {
     expect(props).toBeDefined()
     const version = await getVersionInfo(props)

packages/sdk-codegen-scripts/src/fetchSpec.ts

@@ -289,17 +289,16 @@ export const fetchLookerVersion = async (
   versions?: any,
   options?: Partial<ITransportSettings>
 ) => {
-  let lookerVersion = ''
   if (!versions) {
     try {
       versions = await fetchLookerVersions(props, options)
-      const matches = versions.looker_release_version.match(/^\d+\.\d+/i)
-      lookerVersion = matches[0]
     } catch (e) {
       warn(`Could not retrieve looker release version: ${e.message}`)
+      return ''
     }
   }
-  return lookerVersion
+  const matches = versions.looker_release_version.match(/^\d+\.\d+/i)
+  return matches[0]
 }
 
 /**

packages/sdk-codegen/src/codeGen.ts

@@ -223,6 +223,10 @@ export abstract class CodeGen implements ICodeGen {
     return this.itself ? `${this.itself}.${value}` : value
   }
 
+  typeProperties(type: IType) {
+    return Object.values(type.properties)
+  }
+
   declareType(indent: string, type: IType) {
     const bump = this.bumper(indent)
     const props: string[] = []
@@ -235,7 +239,7 @@ export abstract class CodeGen implements ICodeGen {
         )
         propertyValues = props.join(this.enumDelimiter)
       } else {
-        Object.values(type.properties).forEach((prop) =>
+        this.typeProperties(type).forEach((prop) =>
           props.push(this.declareProperty(bump, prop))
         )
         propertyValues = props.join(this.propDelimiter)

packages/sdk-codegen/src/python.gen.spec.ts

@@ -50,11 +50,17 @@ describe('python generator', () => {
     it('has a models prologue', () => {
       expect(gen.modelsPrologue('')).toEqual(`
 # NOTE: Do not edit this file generated by Looker SDK Codegen
-import attr
 import datetime
 import enum
 from typing import Any, MutableMapping, Optional, Sequence
 
+try:
+    from typing import ForwardRef  # type: ignore
+except ImportError:
+    from typing import _ForwardRef as ForwardRef  # type: ignore
+
+import attr
+
 from looker_sdk.rtl import model
 from looker_sdk.rtl import serialize as sr
 
@@ -76,21 +82,26 @@ class LookerSDK(api_methods.APIMethods):
 `)
     })
     it('has a models epilogue', () => {
+      const type = apiTestModel.types.LookmlModelExploreJoins
+      gen.declareType(indent, type)
       expect(gen.modelsEpilogue('')).toEqual(`
 
 # The following cattrs structure hook registrations are a workaround
 # for https://github.com/Tinche/cattrs/pull/42 Once this issue is resolved
 # these calls will be removed.
 
 import functools  # noqa:E402
-from typing import Any
-try:
-    from typing import ForwardRef  # type: ignore
-except ImportError:
-    from typing import _ForwardRef as ForwardRef  # type: ignore
-
-structure_hook = functools.partial(sr.structure_hook, globals(), sr.converter)
 
+forward_ref_structure_hook = functools.partial(sr.forward_ref_structure_hook, globals(), sr.converter)
+translate_keys_structure_hook = functools.partial(sr.translate_keys_structure_hook, sr.converter)
+sr.converter.register_structure_hook(
+    ForwardRef("LookmlModelExploreJoins"),  # type: ignore
+    forward_ref_structure_hook  # type:ignore
+)
+sr.converter.register_structure_hook(
+    LookmlModelExploreJoins,  # type: ignore
+    translate_keys_structure_hook  # type:ignore
+)
 `)
     })
     it('does not have a methods epilogue', () => {
@@ -450,14 +461,13 @@ return response`
     })
   })
 
-  // TODO reinstate these tests after package dependencies are updated correctly
   describe('type creation', () => {
     it('with arrays and hashes', () => {
       const type = apiTestModel.types.Workspace
       const actual = gen.declareType(indent, type)
       expect(type.properties.id.type.name).toEqual('string')
       expect(actual).toEqual(`
-@attr.s(auto_attribs=True, kw_only=True)
+@attr.s(auto_attribs=True, init=False)
 class Workspace(model.Model):
     """
     Attributes:
@@ -467,14 +477,159 @@ class Workspace(model.Model):
     """
     can: Optional[MutableMapping[str, bool]] = None
     id: Optional[str] = None
-    projects: Optional[Sequence["Project"]] = None`)
+    projects: Optional[Sequence["Project"]] = None
+
+    def __init__(self, *,
+            can: Optional[MutableMapping[str, bool]] = None,
+            id: Optional[str] = None,
+            projects: Optional[Sequence["Project"]] = None):
+        self.can = can
+        self.id = id
+        self.projects = projects`)
+    })
+    it('re-orders required props to top', () => {
+      const type = apiTestModel.types.CreateDashboardFilter
+      const actual = gen.declareType(indent, type)
+      expect(actual).toEqual(`
+@attr.s(auto_attribs=True, init=False)
+class CreateDashboardFilter(model.Model):
+    """
+    Attributes:
+        dashboard_id: Id of Dashboard
+        name: Name of filter
+        title: Title of filter
+        type: Type of filter: one of date, number, string, or field
+        id: Unique Id
+        default_value: Default value of filter
+        model: Model of filter (required if type = field)
+        explore: Explore of filter (required if type = field)
+        dimension: Dimension of filter (required if type = field)
+        field: Field information
+        row: Display order of this filter relative to other filters
+        listens_to_filters: Array of listeners for faceted filters
+        allow_multiple_values: Whether the filter allows multiple filter values
+        required: Whether the filter requires a value to run the dashboard
+        ui_config: The visual configuration for this filter. Used to set up how the UI for this filter should appear.
+    """
+    dashboard_id: str
+    name: str
+    title: str
+    type: str
+    id: Optional[str] = None
+    default_value: Optional[str] = None
+    model: Optional[str] = None
+    explore: Optional[str] = None
+    dimension: Optional[str] = None
+    field: Optional[MutableMapping[str, Any]] = None
+    row: Optional[int] = None
+    listens_to_filters: Optional[Sequence[str]] = None
+    allow_multiple_values: Optional[bool] = None
+    required: Optional[bool] = None
+    ui_config: Optional[MutableMapping[str, Any]] = None
+
+    def __init__(self, *,
+            dashboard_id: str,
+            name: str,
+            title: str,
+            type: str,
+            id: Optional[str] = None,
+            default_value: Optional[str] = None,
+            model: Optional[str] = None,
+            explore: Optional[str] = None,
+            dimension: Optional[str] = None,
+            field: Optional[MutableMapping[str, Any]] = None,
+            row: Optional[int] = None,
+            listens_to_filters: Optional[Sequence[str]] = None,
+            allow_multiple_values: Optional[bool] = None,
+            required: Optional[bool] = None,
+            ui_config: Optional[MutableMapping[str, Any]] = None):
+        self.dashboard_id = dashboard_id
+        self.name = name
+        self.title = title
+        self.type = type
+        self.id = id
+        self.default_value = default_value
+        self.model = model
+        self.explore = explore
+        self.dimension = dimension
+        self.field = field
+        self.row = row
+        self.listens_to_filters = listens_to_filters
+        self.allow_multiple_values = allow_multiple_values
+        self.required = required
+        self.ui_config = ui_config`)
+    })
+    it('with translated keywords', () => {
+      const type = apiTestModel.types.LookmlModelExploreJoins
+      const actual = gen.declareType(indent, type)
+      // note the "from_" property
+      expect(actual).toEqual(`
+@attr.s(auto_attribs=True, init=False)
+class LookmlModelExploreJoins(model.Model):
+    """
+    Attributes:
+        name: Name of this join (and name of the view to join)
+        dependent_fields: Fields referenced by the join
+        fields: Fields of the joined view to pull into this explore
+        foreign_key: Name of the dimension in this explore whose value is in the primary key of the joined view
+        from_: Name of view to join
+        outer_only: Specifies whether all queries must use an outer join
+        relationship: many_to_one, one_to_one, one_to_many, many_to_many
+        required_joins: Names of joins that must always be included in SQL queries
+        sql_foreign_key: SQL expression that produces a foreign key
+        sql_on: SQL ON expression describing the join condition
+        sql_table_name: SQL table name to join
+        type: The join type: left_outer, full_outer, inner, or cross
+        view_label: Label to display in UI selectors
+    """
+    name: Optional[str] = None
+    dependent_fields: Optional[Sequence[str]] = None
+    fields: Optional[Sequence[str]] = None
+    foreign_key: Optional[str] = None
+    from_: Optional[str] = None
+    outer_only: Optional[bool] = None
+    relationship: Optional[str] = None
+    required_joins: Optional[Sequence[str]] = None
+    sql_foreign_key: Optional[str] = None
+    sql_on: Optional[str] = None
+    sql_table_name: Optional[str] = None
+    type: Optional[str] = None
+    view_label: Optional[str] = None
+
+    def __init__(self, *,
+            name: Optional[str] = None,
+            dependent_fields: Optional[Sequence[str]] = None,
+            fields: Optional[Sequence[str]] = None,
+            foreign_key: Optional[str] = None,
+            from_: Optional[str] = None,
+            outer_only: Optional[bool] = None,
+            relationship: Optional[str] = None,
+            required_joins: Optional[Sequence[str]] = None,
+            sql_foreign_key: Optional[str] = None,
+            sql_on: Optional[str] = None,
+            sql_table_name: Optional[str] = None,
+            type: Optional[str] = None,
+            view_label: Optional[str] = None):
+        self.name = name
+        self.dependent_fields = dependent_fields
+        self.fields = fields
+        self.foreign_key = foreign_key
+        self.from_ = from_
+        self.outer_only = outer_only
+        self.relationship = relationship
+        self.required_joins = required_joins
+        self.sql_foreign_key = sql_foreign_key
+        self.sql_on = sql_on
+        self.sql_table_name = sql_table_name
+        self.type = type
+        self.view_label = view_label`)
     })
     it('with refs, arrays and nullable', () => {
       const type = apiTestModel.types.ApiVersion
       expect(type.properties.looker_release_version.type.name).toEqual('string')
       const actual = gen.declareType(indent, type)
       expect(actual).toEqual(`
-@attr.s(auto_attribs=True, kw_only=True)
+@attr.s(auto_attribs=True, init=False)
 class ApiVersion(model.Model):
     """
     Attributes:
@@ -484,7 +639,15 @@ class ApiVersion(model.Model):
     """
     looker_release_version: Optional[str] = None
     current_version: Optional["ApiVersionElement"] = None
-    supported_versions: Optional[Sequence["ApiVersionElement"]] = None`)
+    supported_versions: Optional[Sequence["ApiVersionElement"]] = None
+
+    def __init__(self, *,
+            looker_release_version: Optional[str] = None,
+            current_version: Optional["ApiVersionElement"] = None,
+            supported_versions: Optional[Sequence["ApiVersionElement"]] = None):
+        self.looker_release_version = looker_release_version
+        self.current_version = current_version
+        self.supported_versions = supported_versions`)
     })
 
     function checkMappedType(
@@ -513,6 +676,43 @@ class PermissionType(enum.Enum):
       expect(actual).toEqual(expected)
     })
 
+    it('needs __annotations__', () => {
+      const type = apiTestModel.types.RequiredResponseWithSingleEnum
+      expect(type).toBeDefined()
+      const actual = gen.declareType('', type)
+      const expected = `
+@attr.s(auto_attribs=True, init=False)
+class RequiredResponseWithSingleEnum(model.Model):
+    """
+    Attributes:
+        query_id: Id of query to run
+        result_format: Desired async query result format. Valid values are: "inline_json", "json", "json_detail", "json_fe", "csv", "html", "md", "txt", "xlsx", "gsxml".
+        user:
+        roles: Roles assigned to group
+    """
+    query_id: int
+    result_format: "ResultFormat"
+    user: "UserPublic"
+    roles: Optional[Sequence["Role"]] = None
+    __annotations__ = {
+        "query_id": int,
+        "result_format": ForwardRef("ResultFormat"),
+        "user": ForwardRef("UserPublic"),
+        "roles": Optional[Sequence["Role"]]
+    }
+
+    def __init__(self, *,
+            query_id: int,
+            result_format: "ResultFormat",
+            user: "UserPublic",
+            roles: Optional[Sequence["Role"]] = None):
+        self.query_id = query_id
+        self.result_format = result_format
+        self.user = user
+        self.roles = roles`
+      expect(actual).toEqual(expected)
+    })
+
     it('input models', () => {
       // TODO this side-effect should NOT be required for the test
       // type declarations should be atomic w/o side-effect requirements
@@ -536,7 +736,7 @@ class PermissionType(enum.Enum):
       })
       const actual = gen.declareType(indent, inputType)
       expect(actual).toEqual(`
-@attr.s(auto_attribs=True, kw_only=True, init=False)
+@attr.s(auto_attribs=True, init=False)
 class WriteMergeQuery(model.Model):
     """
     Dynamically generated writeable type for MergeQuery removes properties:
@@ -578,7 +778,7 @@ can, id, result_maker_id
       const childInputType = apiTestModel.types.MergeQuerySourceQuery
       const childActual = gen.declareType(indent, childInputType)
       expect(childActual).toEqual(`
-@attr.s(auto_attribs=True, kw_only=True, init=False)
+@attr.s(auto_attribs=True, init=False)
 class MergeQuerySourceQuery(model.Model):
     """
     Attributes:
@@ -601,7 +801,7 @@ class MergeQuerySourceQuery(model.Model):
       const grandChildInputType = apiTestModel.types.MergeFields
       const grandChildActual = gen.declareType(indent, grandChildInputType)
       expect(grandChildActual).toEqual(`
-@attr.s(auto_attribs=True, kw_only=True, init=False)
+@attr.s(auto_attribs=True, init=False)
 class MergeFields(model.Model):
     """
     Attributes: