Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ConfigConnector configuration option (beta) #547

Merged
merged 1 commit into from May 29, 2020

Conversation

redbaron
Copy link
Contributor

No description provided.

@redbaron redbaron requested review from bharathkkb, Jberlinsky and a team as code owners May 28, 2020 16:56
Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

@redbaron thanks for the PR! You will need to run make build after making changes to the template for it to propagate to all our beta modules.

@redbaron
Copy link
Contributor Author

redbaron commented May 28, 2020

@bharathkkb , thanks, I did just that, but still no luck :(

@bharathkkb
Copy link
Member

@redbaron looks like the main issue is that only the beta modules provider version was bumped to 3.23.0. For integration tests for those modules, we use the examples, so we will need to update the providers for all the examples that use the beta provider. Example.
Here is another example PR that did something similar: #528

After this, you can also run the lint test locally by doing make docker_test_lint.

@redbaron
Copy link
Contributor Author

@bharathkkb , thanks, will do. It's a bit inconvenient that external contributors can't see CI logs :(

@bharathkkb
Copy link
Member

@redbaron absolutely agree, we are working on something that will let external contributors see CI logs soon 😃
Look like a test if failing. We need to add"configConnectorConfig"=>{} to list of expected addons for this test, like we have for kalmConfig:

@redbaron
Copy link
Contributor Author

@bharathkkb , any idea what it might be this time? :)

@bharathkkb
Copy link
Member

bharathkkb commented May 29, 2020

error doesnt seem related, this LGTM. I am re running to check.

@morgante morgante merged commit 672adf9 into terraform-google-modules:master May 29, 2020
@redbaron redbaron deleted the config-connector branch May 30, 2020 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants