Remove addl properties

[fbertsch]

Fixes #22

[acmiyaguchi]

Do you have a concrete example of what was happening in #22? I'm a bit unclear on what the behavior of filling in the probe section is before and after this patch.

[fbertsch]

@acmiyaguchi generally the problem was that previously, if were filling this in:

{
    "metrics": {
        "type": "object",
        "additionalProperties": { "type": "integer" }
    }
}

then we would end up with (for e.g. 1 probe):

{
    "metrics": {
        "type": "object",
        "properties": {
            "probe-1": { "type": "integer" },
        }
        "additionalProperties": { "type": "integer" }
    }
}

And the transpiler would do the correct thing, and ignore the additionalProperties field, giving precedence to the struct type.

However, if there were no fill-in probes, we would end up with something like:

{
    "metrics": {
        "type": "object",
        "additionalProperties": { "type": "integer" }
    }
}

Which the transpiler would see as the pseudo-map type, resulting in a schema element of STRUCT<key_value ARRAY<STRUCT<key STRING, value INT64>>> (or similar). However, when we eventually did have probes to fill-in, the type changes and is not schema evolution compatible.

With this change, the result of no probes to fill-in would be:

{
    "metrics": {
        "type": "object",
        "additionalProperties": False
    }
}

...which, upon inspection, still returns this from the transpiler:

[
  {
    "mode": "NULLABLE",
    "name": "metrics",
    "type": "STRING"
  }
]

Hmm, it seems to me the transpiler should output an empty row here, do you agree? We'd need to check that that is acceptable for BQ.

[acmiyaguchi]

The type is a string because an empty object is not a well defined structure. There needs to be at least one field, even if it's just a dummy/placeholder value. I think instead, you should consider using the propogate=True method to drop the object entirely from the schema. We should also consider adding this as an option to the schema transpiler, since it might be desirable to omit rather than cast in this situation.

[fbertsch]

@acmiyaguchi that seems reasonable as well. I think an empty struct type would be most preferable (since it would match the expected schema of the ping) but deleting it would be second-best.

[acmiyaguchi]

Looking at the BigQuery docs again, it looks like it is possible to have a empty struct.

I also think that we might want to set additionalProperties to true instead of false, otherwise validation will definitely fail.

[fbertsch]

I won't be using these for validation, currently we'll be using the generic glean schema for validation and the strict one for the BQ tables. We can update this later if that proves too lenient.

[fbertsch]

Given that BQ does not support empty structs, we'll have to remove these from the schema instead.

[fbertsch]

@acmiyaguchi this now deletes additionalProperties and propagates that deletion. RFAL

[acmiyaguchi]

Looks good to me.