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

Use log analytics retention policy for diagnostics #328

Conversation

stefpiatek
Copy link
Collaborator

Resolves 320

What is being addressed

When building any new resources that have a retention_policy in their diagnostic settings there is now an error returned from azure:

Message="Diagnostic settings does not support retention for new diagnostic settings."

How is this addressed

  • Remove retention_policy blocks from azure monitor diagnostic settings
  • Falling back to the log analytics retention policy

@stefpiatek
Copy link
Collaborator Author

/test

@jjgriff93
Copy link
Member

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/UCLH-Foundry/FlowEHR/actions/runs/6224972382 (with refid f195aea0)

(in response to this comment from @stefpiatek)

1 similar comment
@jjgriff93
Copy link
Member

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/UCLH-Foundry/FlowEHR/actions/runs/6224972382 (with refid f195aea0)

(in response to this comment from @stefpiatek)

Copy link

@marrobi marrobi left a comment

Choose a reason for hiding this comment

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

LGTM

@stefpiatek
Copy link
Collaborator Author

/test

@jjgriff93
Copy link
Member

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/UCLH-Foundry/FlowEHR/actions/runs/6296819382 (with refid f195aea0)

(in response to this comment from @stefpiatek)

@stefpiatek stefpiatek merged commit 3d26dd4 into SAFEHR-data:main Sep 25, 2023
2 checks passed
@stefpiatek stefpiatek deleted the stefpiatek/log-analystics-retention-policy branch September 25, 2023 08:54
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