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

DEVOPS-1990 Update to go 1.16 #151

Merged
merged 2 commits into from
Jul 30, 2024
Merged

DEVOPS-1990 Update to go 1.16 #151

merged 2 commits into from
Jul 30, 2024

Conversation

bio-boris
Copy link
Contributor

@bio-boris bio-boris commented Jul 30, 2024

  • Attempt to fix crash from http request
{"log":"panic: runtime error: slice bounds out of range\r\n","stream":"stdout","time":"2024-07-26T22:32:50.680661674Z"}

from

This is where it's happening https://github.com/kbase/blobstore/blob/1cc0178ee0a3814aef917b5d4882d52565911494/filestore/s3.go#L151

[s3.go](https://github.com/kbase/blobstore/blob/1cc0178ee0a3814aef917b5d4882d52565911494/filestore/s3.go)
        return nil, errors.New("s3 store request: " + errstr)

Attempted solution is to upgrade the httplib

Copy link

codecov bot commented Jul 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.83%. Comparing base (1cc0178) to head (1fd4d89).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #151   +/-   ##
========================================
  Coverage    90.83%   90.83%           
========================================
  Files           14       14           
  Lines         2073     2073           
========================================
  Hits          1883     1883           
  Misses         157      157           
  Partials        33       33           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bio-boris bio-boris requested a review from MrCreosote July 30, 2024 16:32
Copy link
Member

@MrCreosote MrCreosote left a comment

Choose a reason for hiding this comment

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

Other than the loose pinning not much to review here. I assume you're not considering deploying this on Friday and we'd let it sit on CI for a while?

Dockerfile Outdated
@@ -1,4 +1,4 @@
FROM golang:1.12.5-alpine3.9 as build
FROM golang:1.16-alpine as build
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
FROM golang:1.16-alpine as build
FROM golang:1.16.15-alpine3.15 as build

Is there a reason you're using a looser pinning?

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 just picked one at random. I doubt they will change after 2 years but its possible. I don't really know how they decide to tag images.
Should we do the following instead?

FROM golang@sha256:9743f230f26d1e300545f0330fd4a514f554c535d967563ee77bf634906502b6 as build
# This golang:1.16-alpine3.15 was tagged Mar 3, 2022 at 9:02 pm

Copy link
Member

Choose a reason for hiding this comment

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

I doubt it too but explicit is better than implicit. I think the version tags are good enough and more readable than a hash

@MrCreosote
Copy link
Member

Given we've seen this problem twice in 5 years we're going to have a hard time confirming it's fixed

Co-authored-by: MrCreosote <MrCreosote@users.noreply.github.com>
@bio-boris bio-boris merged commit 5516bab into develop Jul 30, 2024
12 checks passed
@MrCreosote
Copy link
Member

forgot to mention we'll need release notes and a version bump at some point

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