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

Fix app config reporter skip sync configuration on adding app #3647

Merged
merged 2 commits into from
May 23, 2022

Conversation

khanhtc1202
Copy link
Member

What this PR does / why we need it:

Previously, piped can only update the application configuration when the repository which contains that app has new commits after the application be registered to the pipecd database. In case of the app configuration had existed before the app be added, the new registered application configuration can not be synced. This PR updates the logic on appconfigreporter package to ensure the piped only skip syncing applications in case of application's last commit has been scanned, not in case of the application containing repository's last commit has been scanned as currently.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

Fix application's configuration is not synced on registering to pipecd control plane

@pipecd-bot
Copy link
Collaborator

KAPETANIOS

Failed while validating Kapetanios configuration.

Details
Error: rpc error: code = NotFound desc = not found

Copy link
Member

@knanao knanao left a comment

Choose a reason for hiding this comment

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

Looks neats🎉

pkg/app/piped/appconfigreporter/appconfigreporter.go Outdated Show resolved Hide resolved
Copy link
Member

@nghialv nghialv left a comment

Choose a reason for hiding this comment

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

Nice improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants