Add docs for interactive example #4641

metonymic-smokey · 2021-09-06T04:28:29Z

This PR fixes #4639

This PR makes running the interactive example slightly more intuitive by adding the steps used to run it and making the message more intuitive.
It also fixes a small broken link in the docs.

I added CHANGELOG entry for this change.
Change is not relevant to the end user.

Changes

Changed "uncomment" in the message to "comment".
Changed link in Vertical Compaction section in compact.md

Verification

The steps to run the example work as expected and run the test.

Signed-off-by: metonymic-smokey <[email protected]>

metonymic-smokey · 2021-09-06T04:29:01Z

@yeya24 pls review this PR when you can!
Thanks!

yeya24

LGTM! Thanks for the contribution.

yeya24 · 2021-09-07T05:09:37Z

~~@metonymic-smokey Can you run make docs command? I guess that should fix the CI.~~
CI failure not related to this pr.

rahulii · 2021-09-07T07:39:21Z

While running the interactive test, one of the potential errors is Unable to find image 'thanos:latest' locally which is solved by running make docker
@yeya24 do you think we should add it in documentation?
LGTM otherwise!

tutorials/interactive-example/README.md

akanshat · 2021-09-07T07:55:34Z

It looks good to me. LGTM!

onprem

I just have some small suggestions, everything else LGTM. Thanks for taking care of this!

tutorials/interactive-example/README.md

onprem · 2021-09-07T07:59:05Z

tutorials/interactive-example/README.md

@@ -0,0 +1,8 @@
+# Interactive Example


I would prefer if we can have this documentation in the CONTRIBUTING.md file as a section. That way it'll be easier to find by new contributors.

This is just my opinion so I'd love to know WDYT @yeya24 @metonymic-smokey

I think CONTRIBUTING.md could have a link to this file?

Yes, that sounds good. Just make sure to put the complete GitHub URL (like https://github.com/thanos-io/thanos/blob/main/examples/interactive/README.md) otherwise it would get 404'd on website.

@onprem done in the latest commit!

@onprem This link is causing the docs check to fail since the link will become valid only when the PR is merged to main. Is there something I can do to fix it?

Ah, no worries. I can go ahead and merge this as the link will start working after the merge.

onprem · 2021-09-07T08:03:48Z

While running the interactive test, one of the potential errors is Unable to find image 'thanos:latest' locally which is solved by running make docker
@yeya24 do you think we should add it in documentation?
LGTM otherwise!

Yes, that'd be nice. We can also have a Makefile target for running the interactive example, with docker target as dependency.

onprem · 2021-09-07T08:04:26Z

Sorry, closed the PR by mistake.

rahulii · 2021-09-07T08:12:38Z

While running the interactive test, one of the potential errors is Unable to find image 'thanos:latest' locally which is solved by running make docker
@yeya24 do you think we should add it in documentation?
LGTM otherwise!

Yes, that'd be nice. We can also have a Makefile target for running the interactive example, with docker target as dependency.

@onprem should i take this ?
cc @squat @GiedriusS

GiedriusS · 2021-09-07T08:27:38Z

While running the interactive test, one of the potential errors is Unable to find image 'thanos:latest' locally which is solved by running make docker
@yeya24 do you think we should add it in documentation?
LGTM otherwise!

Yes, that'd be nice. We can also have a Makefile target for running the interactive example, with docker target as dependency.

@onprem should i take this ?
cc @squat @GiedriusS

Yep, it would be great because currently, it is hard to find 👍

Signed-off-by: metonymic-smokey <[email protected]>

onprem

LGTM 🚀

Thanks!

bwplotka

Thanks 🤗

bwplotka · 2021-09-13T15:19:08Z

examples/interactive/interactive_test.go

@@ -89,7 +89,7 @@ func createData() (perr error) {
 // TestReadOnlyThanosSetup runs read only Thanos setup that has data from `maxTimeFresh - 2w` to `maxTimeOld`, with extra monitoring and tracing for full playground experience.
 // Run with test args `-test.timeout 9999m`.
 func TestReadOnlyThanosSetup(t *testing.T) {
-	t.Skip("This is interactive test - it will until you will kill it or curl 'finish' endpoint. Uncomment and run as normal test to use it!")
+	t.Skip("This is interactive test - it will until you will kill it or curl 'finish' endpoint. Comment and run as normal test to use it!")


t.Skip("This is interactive test - it will until you will kill it or curl 'finish' endpoint. Comment and run as normal test to use it!")

... it will run until...

@bwplotka sorry for the delay, I shall open a PR fixing this soon.

* fixed broken compaction link Signed-off-by: metonymic-smokey <[email protected]> * fixed message in example Signed-off-by: metonymic-smokey <[email protected]> * added README for interactive example Signed-off-by: metonymic-smokey <[email protected]> * refer to issue about MacOS support Signed-off-by: metonymic-smokey <[email protected]> * added reference in CONTRIBUTING.md Signed-off-by: metonymic-smokey <[email protected]>

metonymic-smokey and others added 4 commits September 5, 2021 07:46

fixed broken compaction link

f306519

Signed-off-by: metonymic-smokey <[email protected]>

Merge branch 'thanos-io:main' into docs-fix

b672b77

fixed message in example

1b3e108

Signed-off-by: metonymic-smokey <[email protected]>

added README for interactive example

0d75c1b

Signed-off-by: metonymic-smokey <[email protected]>

yeya24 previously approved these changes Sep 7, 2021

View reviewed changes

yeya24 reviewed Sep 7, 2021

View reviewed changes

tutorials/interactive-example/README.md Show resolved Hide resolved

onprem reviewed Sep 7, 2021

View reviewed changes

onprem closed this Sep 7, 2021

onprem reopened this Sep 7, 2021

refer to issue about MacOS support

8986c13

Signed-off-by: metonymic-smokey <[email protected]>

metonymic-smokey dismissed yeya24’s stale review via 8986c13 September 7, 2021 08:29

added reference in CONTRIBUTING.md

2fa7614

Signed-off-by: metonymic-smokey <[email protected]>

metonymic-smokey requested review from yeya24 and onprem September 7, 2021 12:04

Merge branch 'thanos-io:main' into docs-fix

f2326fd

onprem approved these changes Sep 7, 2021

View reviewed changes

onprem enabled auto-merge (squash) September 7, 2021 12:09

onprem disabled auto-merge September 7, 2021 13:13

onprem merged commit 1205a22 into thanos-io:main Sep 7, 2021

bwplotka reviewed Sep 13, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add docs for interactive example #4641

Add docs for interactive example #4641

metonymic-smokey commented Sep 6, 2021 •

edited

Loading

metonymic-smokey commented Sep 6, 2021

yeya24 left a comment

yeya24 commented Sep 7, 2021 •

edited

Loading

rahulii commented Sep 7, 2021

akanshat commented Sep 7, 2021

onprem left a comment

onprem Sep 7, 2021

onprem Sep 7, 2021

metonymic-smokey Sep 7, 2021

onprem Sep 7, 2021

metonymic-smokey Sep 7, 2021

metonymic-smokey Sep 7, 2021

onprem Sep 7, 2021

onprem commented Sep 7, 2021 •

edited

Loading

onprem commented Sep 7, 2021

rahulii commented Sep 7, 2021

GiedriusS commented Sep 7, 2021

onprem left a comment

bwplotka left a comment

bwplotka Sep 13, 2021

metonymic-smokey Sep 18, 2021

Add docs for interactive example #4641

Add docs for interactive example #4641

Conversation

metonymic-smokey commented Sep 6, 2021 • edited Loading

Changes

Verification

metonymic-smokey commented Sep 6, 2021

yeya24 left a comment

Choose a reason for hiding this comment

yeya24 commented Sep 7, 2021 • edited Loading

rahulii commented Sep 7, 2021

akanshat commented Sep 7, 2021

onprem left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onprem commented Sep 7, 2021 • edited Loading

onprem commented Sep 7, 2021

rahulii commented Sep 7, 2021

GiedriusS commented Sep 7, 2021

onprem left a comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metonymic-smokey commented Sep 6, 2021 •

edited

Loading

yeya24 commented Sep 7, 2021 •

edited

Loading

onprem commented Sep 7, 2021 •

edited

Loading