[improvement] add test template to verify assessment flag usage

[vladislav-sidorovich]

It's very simple forget to add '--assessment' flag to connector. The problem is that the mistake will be find only during uploading zip to server.

So, the idea is to explicitly mark each connector if assessment flag is required or not for the connector. In addition to that the test will verify that connectors are aligned with the annotations.

Please help to double check if the connectors marked properly by RespectsArgumentAssessment or AvoidArgumentAssessment.

It should be at least align with our public doc: https://cloud.google.com/bigquery/docs/migration-assessment#teradata_1

Note: the test is failed because the validation logic doesn't exist for all the connector. I would prefer first define the list of connectors and add the validation logic later.

[misolt]

Many connectors don't have this annotation, despite using the --assessment flag and they will not be found by this test.

[vladislav-sidorovich]

It's true. We need an additional test to verify that developer explicitly chased to use or do not use the annotation.

[kpiotrowski]

What about the connectors that are shared by assessment and translation? Depending on the use case they will need --assessment flag or not. This is true for most of the "metadata" connectors supported by the assessment. Additionally, some query logs connectors are shared by assessment and lineage.

[vladislav-sidorovich]

What about the connectors that are shared by assessment and translation? Depending on the use case they will need --assessment flag or not. This is true for most of the "metadata" connectors supported by the assessment. Additionally, some query logs connectors are shared by assessment and lineage.

In this case such connectors should be able accept and process the flag. But I think we can go case by case.

@shevek-google what do you think?

[misolt]

Alternative solution:

• Add an enum getter in Connector, e.g. getAssessmentSupport()
• The enum represents the connector's --assesssment flag expectations, with the values REQUIRED, FORBIDDEN, OPTIONAL.
• Implement a default in Connector to return FORBIDDEN.
• Check and enforce it in shared dumper code

This has a few benefits:

• Can be overridden as abstract in an abstract connector to force explicit handling by subclasses.
• No changes to connectors that assessment doesn't use, except more strict validation.
• It's easy to look up the connectors that allow the flag.
• Optionally, ConnectorArguments can be changed to check if a connector asking about the assessment flag makes sense - for REQUIRED and FORBIDDEN it doesn't.

[github-actions]

This PR is stale because it has been open 35 days with no activity. Remove "stale" label or comment or this will be closed in 14 days.