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 snake case on Azure Storage config #1883

Merged

Conversation

faustodavid
Copy link
Contributor

What this PR does:
Use snake case on Azure Storage config

Which issue(s) this PR fixes:
Fixes #1879

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Should I update the docker-compose azure example now or after Tempo 2.0 is released?

Signed-off-by: Fausto David Suarez Rosario <[email protected]>
Signed-off-by: Fausto David Suarez Rosario <[email protected]>
Signed-off-by: Fausto David Suarez Rosario <[email protected]>
@faustodavid faustodavid force-pushed the feature/azure-storage-use-snake-case branch from b7c7d05 to 64d69a4 Compare November 10, 2022 10:54
Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

Thank you for this fix. I've been wanting to do it for awhile, but haven't found the time.

There is also a file we use for a local azure example that will need to be updated:
/example/docker-compose/azure/tempo-azure.yaml

It also looks like you need to run make jsonnet to rebuild the jsonnet files? This requires some local tooling setup that can be kind of annoying if you never use jsonnet. I can run this and push to your branch if it's easier.

Thanks @faustodavid!

@faustodavid
Copy link
Contributor Author

Thank you for this fix. I've been wanting to do it for awhile, but haven't found the time.

There is also a file we use for a local azure example that will need to be updated: /example/docker-compose/azure/tempo-azure.yaml

It also looks like you need to run make jsonnet to rebuild the jsonnet files? This requires some local tooling setup that can be kind of annoying if you never use jsonnet. I can run this and push to your branch if it's easier.

Thanks @faustodavid!

Happy to contribute! :D

@joe-elliott I wasn't sure if I should update the docker-compose example because if somebody tries to run it without compiling Tempo it will fail.

* update azure docker-compose example

Signed-off-by: Fausto David Suarez Rosario <[email protected]>
Signed-off-by: Fausto David Suarez Rosario <[email protected]>
@joe-elliott
Copy link
Member

@joe-elliott I wasn't sure if I should update the docker-compose example because if somebody tries to run it without compiling Tempo it will fail.

This is a good point, but I think we have to keep the examples consistent with tip of main. I'd prefer it fail if you have an older tempo:latest than if you have the most recent tempo:latest

Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

Thanks!

@joe-elliott joe-elliott merged commit 05854f2 into grafana:main Nov 10, 2022
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.

Snake case for Azure backend configuration
2 participants