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 Docs to Show Azure App Configuration subscribePollInterval as ns #3595

Merged
merged 7 commits into from
Jul 27, 2023

Conversation

tjsiron
Copy link
Contributor

@tjsiron tjsiron commented Jul 2, 2023

Description

When investigating the docs for the Azure App Configuration backend for configuration, I noticed the subscribePollInterval parameter is shown in 1ms, 1m, 1h format, but the golang code and the component definition here https://github.com/dapr/components-contrib/blob/master/configuration/azure/appconfig/metadata.yaml are expecting it in nanoseconds. Following the docs as is results in the following error:

level=fatal msg="process component configstore error: [INIT_COMPONENT_FAILURE]: initialization error occurred for configstore (configuration.azure.appconfig/v1): azure appconfig error: can't parse subscribePollInterval field: strconv.Atoi: parsing \"1s\": invalid syntax"

Issue reference

I didn't create one, but I can if needed.

@tjsiron tjsiron requested review from a team as code owners July 2, 2023 23:06
Signed-off-by: Tyler Siron <[email protected]>
@tjsiron tjsiron force-pushed the ts-azure-app-config-subscribe-interval branch from d54b2d6 to b190776 Compare July 2, 2023 23:07
Signed-off-by: Tyler Siron <[email protected]>
@github-actions
Copy link

Stale PR, paging all reviewers

@msfussell
Copy link
Member

This is also a bug and we should fix this to be consistent with intervals type. Given this is alpha we can do the breaking change on the component. https://docs.dapr.io/reference/components-reference/supported-configuration-stores/
So first let's raise a bug to track this in component contrib and second we can take this with a note in the docs here to say this will be updated in the future to be the specified format

@hhunter-ms
Copy link
Collaborator

hhunter-ms commented Jul 11, 2023

@tjsiron could you add a small note to this doc per Mark's comment:

A note in the docs here to say this will be updated in the future to be the specified format

Once added, we can merge this PR until we fix the bug!

Also, please open an issue for this here: https://github.com/dapr/components-contrib/issues

@github-actions
Copy link

Stale PR, paging all reviewers

@msfussell
Copy link
Member

I raised this tracking issue, dapr/components-contrib#2998 however we should push this PR with a comment and raise a new issue in the docs to track this issue when fixed

@github-actions
Copy link

Stale PR, paging all reviewers

@hhunter-ms
Copy link
Collaborator

I raised this tracking issue, dapr/components-contrib#2998 however we should push this PR with a comment and raise a new issue in the docs to track this issue when fixed

I opened a new issue for when the code has been updated.

Copy link
Member

@msfussell msfussell left a comment

Choose a reason for hiding this comment

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

LGTM.

@hhunter-ms hhunter-ms merged commit c1e24c8 into dapr:v1.11 Jul 27, 2023
7 checks passed
@yaron2 yaron2 added this to the 1.12 milestone Oct 11, 2023
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.

4 participants