-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Update go version to go 1.14.3 #18829
Conversation
Pinging @elastic/integrations-services (Team:Services) |
💔 Build FailedExpand to view the summary
Build stats
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
Pinging @elastic/ingest-management (Team:Ingest Management) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change LGTM, @urso any concerns about upgrading from 1.13.10 to 1.14.3 in master and 7.x? quick look at the open issue on Go's repository doesn't show anything problematic?
# OS X 10.11 Mountain Lion is the oldest supported by Go 1.13. | ||
# https://golang.org/doc/go1.13#ports | ||
# OS X 10.11 El Capitan is the oldest supported by Go 1.14. | ||
# https://golang.org/doc/go1.14#ports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
@blakerouse The failures of auditbeat and other beats seems related to theses changes? |
@ph Investigating them now, I know the metricbeat ones are related. |
found = true | ||
} | ||
} | ||
if !found { | ||
assert.Failf(t, "", "expected an error containing 'request canceled (Client.Timeout exceeded'. Got %v", errs) | ||
assert.Failf(t, "", "expected an error containing 'context deadline exceeded (Client.Timeout exceeded'. Got %v", errs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh they changed the returned string vlue.
Go 1.14.0 introduced a regression in the JSON parser. Due to some faulty state handling UTF-8 characters are not always handled correctly, which can result in 'splits' and invalid/mangled characters in input strings. The fix will only be available in go1.14.4. Related issues:
Checking the known issues, the JSON one is the last outstanding issue that can affect us and our users. |
https://github.com/golang/go/issues?q=milestone%3AGo1.14.4+label%3ACherryPickApproved This is indeed a problem, there is no guarantee if it will be shipped before our next major. @blakerouse could we make it work with 1.13.x? |
@ph Yes I can make it work with go 1.13, switching to go 1.14 was the
easiest path, but if its broken I can make it work.
…On Fri, May 29, 2020, 11:16 AM Pier-Hugues Pellerin < ***@***.***> wrote:
Go 1.14.0 introduced a regression in the JSON parser. Due to some faulty
state handling UTF-8 characters are not always handled correctly, which can
result in 'splits' and invalid/mangled characters in input strings. The fix
will only be available in go1.14.4. Related issues:
https://github.com/golang/go/issues?q=milestone%3AGo1.14.4+label%3ACherryPickApproved
This is indeed a problem, there is no guarantee if it will be shipped
before our next major.
@blakerouse <https://github.com/blakerouse> could we make it work with
1.13.x?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18829 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA5QXZ4PTJAWCNGYEHNALLTRT7GUDANCNFSM4NNMLJPA>
.
|
@blakerouse lets do that instead, I think it's worth it so we do not depend on Golang to release a fix. From what I've seen in the 1.14 had a rocky beginning :) |
Closing this, once go 1.14.4 is released we can update then. |
@blakerouse FYI we have an open issue for updating Go to 1.14 with the required tasks: #16566 |
@kvch I used it to create this branch and linked it to that issue. Other than the regressions in go 1.14.0, I think this is close once 1.14.4 is released. I did the removal of |
@blakerouse I've merged removing the vendor folder. Could you please update |
|
@kuisathaverat The branch is in an incorrect state at the moment. Turning off go modules is not the correct solution. We need to install it |
# Conflicts: # .ci/scripts/install-tools.bat # Jenkinsfile # auditbeat/make.bat # dev-tools/mage/fmt.go # dev-tools/mage/settings.go # dev-tools/make/mage-install.mk # filebeat/make.bat # heartbeat/make.bat # libbeat/make.bat # libbeat/scripts/Makefile # make.bat # metricbeat/make.bat # packetbeat/make.bat # winlogbeat/make.bat # x-pack/filebeat/make.bat # x-pack/winlogbeat/make.bat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once changelog is updated and everything is green, merge.
lint is failing. Check for usages of |
Setting |
@blakerouse We do not have vendor folder in our repositry. So go modules is not running in vendor mode.
|
## What does this PR do? This PR adds support for Golang 1.14.4 in Beats. The dependency `golang.org/x/tools` is pinned to `release-branch.go1.14`. The generated Beats no longer contain a vendor folder. ## Why is it important? Go 1.14 has been out for quite some time. It is high time we update. ## Related issues Closes #18829 Closes #16566
This PR adds support for Golang 1.14.4 in Beats. The dependency `golang.org/x/tools` is pinned to `release-branch.go1.14`. The generated Beats no longer contain a vendor folder. Go 1.14 has been out for quite some time. It is high time we update. Closes elastic#18829 Closes elastic#16566 (cherry picked from commit 7580c72)
This PR adds support for Golang 1.14.4 in Beats. The dependency `golang.org/x/tools` is pinned to `release-branch.go1.14`. The generated Beats no longer contain a vendor folder. Go 1.14 has been out for quite some time. It is high time we update. Closes #18829 Closes #16566 (cherry picked from commit 7580c72)
## What does this PR do? This PR adds support for Golang 1.14.4 in Beats. The dependency `golang.org/x/tools` is pinned to `release-branch.go1.14`. The generated Beats no longer contain a vendor folder. ## Why is it important? Go 1.14 has been out for quite some time. It is high time we update. ## Related issues Closes elastic#18829 Closes elastic#16566
What does this PR do?
Updates the go version to go 1.14.4. Removes the
-mod=vendor
through out the make and mage files being that is the new default in go 1.14.Why is it important?
To be on the latest version of go 1.14.
Checklist
I have added tests that prove my fix is effective or that my feature worksCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs