fix: Error on incorrect table configuration#237
Conversation
yevgenypats
left a comment
There was a problem hiding this comment.
Looks good. few nits.
| } | ||
|
|
||
| // return an error if child table is included, but not its parent table | ||
| selectedTables := map[string]bool{} |
There was a problem hiding this comment.
Nit: I think we should just disallow child tables (at least for now until someone will ask for it.)
There was a problem hiding this comment.
Before we disallow child tables, we should indicate what is and isn't a child table in the docs. (Not hard, I can do this in a separate PR). But let's at least allow it to be specified for now, as long as you also specify the parent, otherwise you'll get some surprising behavior.
There was a problem hiding this comment.
In that case, if child table is specified I'll automatically fetch parent tables if we are at it already.
|
|
||
| logger.Debug().Interface("tables", tableNames).Msg("got table names") | ||
|
|
||
| for _, table := range p.tables { |
There was a problem hiding this comment.
If we already validates tables, let's just use tableName and do range _, table := range tableName so the validation logic is in one place
There was a problem hiding this comment.
tableNames is of type []string, while p.tables is of type schema.Tables. We can't range over tableNames, since need the schema.Table here. We could add a lookup for the schema.Table object? I played around with it, but it doesn't seem better
|
@hermanschaaf can you solve the conflict and we can merge? |
🤖 I have created a release *beep* *boop* --- ## [0.12.8](v0.12.7...v0.12.8) (2022-10-07) ### Bug Fixes * Error on incorrect table configuration ([#237](#237)) ([6ad75f5](6ad75f5)) * Exit gracefully on context cancelled ([#252](#252)) ([b4df92e](b4df92e)) * Progressbar should go into stdout ([#250](#250)) ([b8bcdad](b8bcdad)) * Recover panic in table resolver and object resolver flows ([#257](#257)) ([04dba02](04dba02)) * Stop if PreResourceResolver fails ([#251](#251)) ([ee83f8f](ee83f8f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This adds checks for a whole range of misconfigurations that could happen in the tables config.
Some of the main cases handled:
Closes cloudquery/cloudquery#2312