Skip to content

[content-service] Implement UsageReportService.DownloadURL #12335

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

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

easyCZ
Copy link
Member

@easyCZ easyCZ commented Aug 24, 2022

Description

Implements DownloadURL rpc. This is needed to be able to download a usage report form the usage component.

Related Issue(s)

How to test

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview

@easyCZ
Copy link
Member Author

easyCZ commented Aug 24, 2022

/hold for dependency

@@ -32,7 +32,7 @@ const (

var (
// ErrNotFound is returned when an object is not found
ErrNotFound = xerrors.Errorf("not found")
ErrNotFound = fmt.Errorf("not found")
Copy link
Member Author

Choose a reason for hiding this comment

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

Drive-by: xerrors.Errorf is deperacted

@easyCZ easyCZ force-pushed the mp/content-get-download-url branch 2 times, most recently from 6c144ec to 85dc4fb Compare August 25, 2022 07:56
@easyCZ easyCZ force-pushed the mp/content-get-download-url-impl branch 2 times, most recently from 61b286c to 66c18fe Compare August 25, 2022 20:20
@easyCZ
Copy link
Member Author

easyCZ commented Aug 25, 2022

/hold for dependency

@easyCZ easyCZ marked this pull request as ready for review August 25, 2022 20:21
@easyCZ easyCZ requested a review from a team August 25, 2022 20:21
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label Aug 25, 2022
@easyCZ easyCZ changed the title [content-service] Implement UsageReportService.GetDownloadURL [content-service] Implement UsageReportService.DownloadURL Aug 25, 2022
return nil, status.Error(codes.InvalidArgument, "Name is required but got empty.")
}

logger := log.WithField("name", req.Name).
Copy link
Contributor

Choose a reason for hiding this comment

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

usually we just name the variable log in cases like this.

Suggested change
logger := log.WithField("name", req.Name).
log := log.WithField("name", req.Name).

Copy link
Member Author

Choose a reason for hiding this comment

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

log is the package name where the logging is implemented. It's a name clash that we shouldn't allow.

Base automatically changed from mp/content-get-download-url to main August 26, 2022 11:37
@roboquat roboquat added size/XXL and removed size/M labels Aug 26, 2022
@easyCZ easyCZ force-pushed the mp/content-get-download-url-impl branch from 66c18fe to 26e93b8 Compare August 26, 2022 11:41
@roboquat roboquat added size/M and removed size/XXL labels Aug 26, 2022
@easyCZ
Copy link
Member Author

easyCZ commented Aug 26, 2022

dependency landed
/unhold

@roboquat roboquat merged commit cf9e765 into main Aug 26, 2022
@roboquat roboquat deleted the mp/content-get-download-url-impl branch August 26, 2022 12:09
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note-none size/M team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants