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

Fix docs that fail after the fix in v0.45.1 #1284

Merged
merged 4 commits into from
Aug 7, 2023
Merged

Conversation

immavalls
Copy link
Contributor

After the issue solved with grafana/k6#3252, the was incorrect, as it was using v0.43.1. Reported in the forum https://community.grafana.com/t/error-installing-xk6-on-macos-both-using-go-directly-and-docker/100621/3

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

There's a version of the docs published here:

https://mdr-ci.staging.k6.io/docs/refs/pull/1284/merge

It will be deleted automatically in 30 days.


<CodeGroup labels={["Linux", "Mac", "Windows PowerShell", "Windows"]}>

```bash
docker run --rm -u "$(id -u):$(id -g)" -v "${PWD}:/xk6" grafana/xk6 build v0.43.1 \
docker run --rm -u "$(id -u):$(id -g)" -v "${PWD}:/xk6" grafana/xk6 build latest \
Copy link
Contributor

Choose a reason for hiding this comment

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

I would argue to drop latest here and the explanation above.

This way the k6 will build with the highest version requested by the extensions. And won't fail if the latest version happens to be breaking any of them, but none of them request it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was indeed my first attempt, but the command fails if we don't specify latest, with the same errors as if it was building with a previous version... On my mac:

 docker run --rm -e GOOS=darwin -u "$(id -u):$(id -g)" -v "${PWD}:/xk6" \
  grafana/xk6 build \
  --with github.com/mostafa/[email protected] \
  --with github.com/grafana/[email protected]
go: downloading github.com/chromedp/sysutil v1.0.0
k6 imports
	go.k6.io/k6/cmd imports
	github.com/grafana/xk6-output-prometheus-remote/pkg/remotewrite imports
	go.buf.build/grpc/go/prometheus/prometheus: unrecognized import path "go.buf.build/grpc/go/prometheus/prometheus": https fetch: Get "https://go.buf.build/grpc/go/prometheus/prometheus?go-get=1": dial tcp: lookup go.buf.build on 192.168.65.7:53: no such host
2023/08/07 12:33:51 [INFO] Cleaning up temporary folder: /tmp/buildenv_2023-08-07-1233.1381995159

Copy link
Contributor

Choose a reason for hiding this comment

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

Bump the versions of the extensions to one that use v0.45.1
https://github.com/mostafa/xk6-kafka/releases/tag/v0.19.1
https://github.com/grafana/xk6-output-influxdb/releases/tag/v0.4.1

I even wonder if we should not just put the version to latest

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 guess it was the same thing, we wanted to showcase how to add a concrete version. Let me bump those then

Copy link
Contributor

@javaducky javaducky Aug 7, 2023

Choose a reason for hiding this comment

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

Yeah, those explicit versions were to ensure the command would run. Obviously, the transitive dependency issue corrected by v0.45.1 breaks that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing the page to have a couple of examples (one without specifying versions and another with concrete versions that work...now). I'll let you know once it's done for review

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done @mstoykov & @javaducky Let me know if that's better or we still would adjust anything

@javaducky javaducky self-requested a review August 7, 2023 13:22
Copy link
Contributor

@javaducky javaducky left a comment

Choose a reason for hiding this comment

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

Minor suggestion on phrasing, but LGTM!

@immavalls immavalls merged commit f32b958 into main Aug 7, 2023
@immavalls immavalls deleted the fix-extension-build-docs branch August 7, 2023 14:56
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.

3 participants