Repository: spotify/luigi
File: luigi/contrib/bigquery_avro.py (Python)

Code before:

                #Map Support
                if field_type is avro.schema.MapSchema:
                    #directly bypass the key and value attributes in BigQuery Schema
                    bq_map_value_field = field[u'fields'][-1]
                    avro_map_value = avro_field.type.values
                    value_field_type = type(avro_map_value)

                    #Primitive Support
                    # -- Unfortunately the map element doesn't directly have a doc attribute so there is no way to get documentation on the primitive types for the value attribute

                    if value_field_type is avro.schema.EnumSchema:
                        bq_map_value_field[u'description'] = avro_map_value.type.doc

                    if value_field_type is avro.schema.RecordSchema:
                        #Set values description using type's doc
                        bq_map_value_field[u'description'] = avro_map_value.doc

                        #This is jumping into the map value directly and working with that
                        bq_map_value_field[u'fields'] = get_fields_with_description(bq_map_value_field[u'fields'], avro_map_value.fields_dict)

                    if value_field_type is avro.schema.ArraySchema:
                        bq_map_value_field[u'description'] = avro_map_value.items.doc
                        bq_map_value_field[u'fields'] = get_fields_with_description(bq_map_value_field[u'fields'], avro_map_value.items.fields_dict)

                    #Unions nested in maps are not supported
                    if value_field_type is avro.schema.UnionSchema:
                        logger.info("Unions inside of maps are not supported for documentation.")

                    #Nested Maps are not supported
                    if value_field_type is avro.schema.MapSchema:
                        logger.info("Nested Maps are not supported for documentation.")

                new_fields.append(field)
            return new_fields

        field_descriptions = get_fields_with_description(current_bq_schema['schema']['fields'], avro_schema.fields_dict)

        patch = {
            'description': avro_schema.doc,
            'schema': {'fields': field_descriptions, },
        }

        bq_client.tables().patch(projectId=table.project_id,
                                 datasetId=table.dataset_id,
                                 tableId=table.table_id,
                                 body=patch).execute()

    def run(self):
        super(BigQueryLoadAvro, self).run()


Review comment (suggestion):
The comment repeats what the code says and doesn't explain "why". Is it because we leave implementation for later? Then we could explain exactly that, or say "TODO" instead of "not supported". Is it for another reason? It's hard to tell for a new reader of the code.

Code after:
                    for schema in avro_field.type.schemas:
                        if type(schema) is avro.schema.PrimitiveSchema:
                            field[u'description'] = avro_field.doc

                        if type(schema) is avro.schema.RecordSchema:
                            field[u'description'] = schema.doc
                            field[u'fields'] = get_fields_with_description(field[u'fields'], schema.fields_dict)

                            # Support for Enums, Arrays, Maps, and Unions inside of a union is not yet implemented

                # Map Support
                if field_type is avro.schema.MapSchema:
                    field[u'description'] = avro_field.doc

                    # Big Query Avro loader creates artificial key and value attributes in the Big Query schema
                    # ignoring the key and operating directly on the value
                    # https://cloud.google.com/bigquery/data-formats#avro_format
                    bq_map_value_field = field[u'fields'][-1]
                    avro_map_value = avro_field.type.values
                    value_field_type = type(avro_map_value)

                    # Primitive Support: Unfortunately the map element doesn't directly have a doc attribute
                    # so there is no way to get documentation on the primitive types for the value attribute

                    if value_field_type is avro.schema.EnumSchema:
                        bq_map_value_field[u'description'] = avro_map_value.type.doc

                    if value_field_type is avro.schema.RecordSchema:
                        # Set values description using type's doc
                        bq_map_value_field[u'description'] = avro_map_value.doc

                        # This is jumping into the map value directly and working with that
                        bq_map_value_field[u'fields'] = get_fields_with_description(bq_map_value_field[u'fields'], avro_map_value.fields_dict)

                    if value_field_type is avro.schema.ArraySchema:
                        bq_map_value_field[u'description'] = avro_map_value.items.doc
                        bq_map_value_field[u'fields'] = get_fields_with_description(bq_map_value_field[u'fields'], avro_map_value.items.fields_dict)

                        # Support for unions and maps nested inside of a map is not yet implemented

                new_fields.append(field)
            return new_fields

        field_descriptions = get_fields_with_description(current_bq_schema['schema']['fields'], avro_schema.fields_dict)

        patch = {
            'description': avro_schema.doc,
            'schema': {'fields': field_descriptions, },
        }

        bq_client.tables().patch(projectId=table.project_id,