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 Azure support to up space billing get + provider-generic refactor #351

Merged
merged 11 commits into from
Aug 16, 2023

Conversation

branden
Copy link
Contributor

@branden branden commented Aug 11, 2023

Description of your changes

This adds support for Azure blob storage to up space billing get. It also refactors the GCS and S3 implementations to use provider-generic code for generating the report.

Other things worth noting:

  • All provider implementations support reading gzipped data.
  • S3 events can now be grouped by arbitrarily sized windows, like the other providers.
  • The number of S3 objects that can be read in a time window is now unlimited, up from 5000.
  • I gave no consideration to optimizing download speed with concurrency. I'll leave this for a later PR.

This is a big PR, so see commits for logical groupings of changes.

I have:

  • Read and followed Upbound's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR, as appropriate.

How has this code been tested

I ran up space billing get against sample data with each storage provider. I'll update this with more detail shortly.

@branden branden requested a review from a team August 11, 2023 01:44
Copy link
Contributor

@darkmuggle darkmuggle left a comment

Choose a reason for hiding this comment

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

Minior nits, but otherwise LGTM

internal/usage/event/event.go Outdated Show resolved Hide resolved
"github.com/upbound/up/internal/usage/model"
)

var EOF = event.EOF
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this error should be a common error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean, can you elaborate?

@branden branden merged commit 4953a9b into upbound:main Aug 16, 2023
6 checks passed
@branden branden deleted the space-billing-get-azure branch August 16, 2023 23:15
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.

2 participants