Skip to content

Conversation

@erezrokah
Copy link
Member

@erezrokah erezrokah commented Oct 19, 2023

Summary

Fixes https://github.com/cloudquery/cloudquery-issues/issues/690 (internal issue).

Tables.TableNames() is recursive and gets all nested names so we can't use it.

Without the fix each table will have all of descendants under relations instead of only the children (1 level descendants)


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

tables = append(tables, table)
}
return tables, nil
return tables.FilterDfs(opts.Tables, opts.SkipTables, opts.SkipDependentTables)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed so we set parents implicitly via Copy (see also cloudquery/cloudquery#14637)

@github-actions github-actions bot added fix and removed fix labels Oct 19, 2023
@hermanschaaf
Copy link
Member

@erezrokah There's a test failure FYI (expected 2 tables but got 3)

@github-actions
Copy link

github-actions bot commented Oct 19, 2023

⏱️ Benchmark results

  • Glob-8 ns/op: 246.9

@kodiakhq kodiakhq bot merged commit 3d4ebe0 into cloudquery:main Oct 19, 2023
@erezrokah erezrokah deleted the fix/package_tables branch October 19, 2023 11:21
kodiakhq bot pushed a commit that referenced this pull request Oct 19, 2023
🤖 I have created a release *beep* *boop*
---


## [4.16.1](v4.16.0...v4.16.1) (2023-10-19)


### Bug Fixes

* **package:** Only return one level down of relations when writing `tables.json` ([#1321](#1321)) ([3d4ebe0](3d4ebe0))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants