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: Improve docs for min and max table offsets #14890

Merged

Conversation

salvacorts
Copy link
Contributor

What this PR does / why we need it:
The docs for min and max offsets are a bit difficult to reason about. This PR improves them with examples and sets the default min offset to 0 (build from today), and the max to 1 (build till tomorrow).

We also add a test for the tables iterator.

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@salvacorts salvacorts changed the title Improve docs for min and max table offsets fix: Improve docs for min and max table offsets Nov 13, 2024
@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Nov 13, 2024
@salvacorts salvacorts marked this pull request as ready for review November 13, 2024 11:48
@salvacorts salvacorts requested a review from a team as a code owner November 13, 2024 11:48
docs/sources/shared/configuration.md Outdated Show resolved Hide resolved
# CLI flag: -bloom-build.planner.max-table-offset
[max_table_offset: <int> | default = 2]
[max_table_offset: <int> | default = 1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we increase the default to 7? I think this would make sense in the broader context

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'm hesitant to do that until we get more confident about how good the build process scales. If data doesn't change as often, building today + tomorrow should work more most of the cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok with that

salvacorts and others added 2 commits November 13, 2024 12:58
Co-authored-by: Christian Haudum <christian.haudum@gmail.com>
@salvacorts salvacorts merged commit fd9d332 into main Nov 13, 2024
58 checks passed
@salvacorts salvacorts deleted the salvacorts/fix-default-min-max-table-offsets-and-docs branch November 13, 2024 12:30
@loki-gh-app
Copy link
Contributor

loki-gh-app bot commented Nov 14, 2024

The backport to k227 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-14890-to-k227 origin/k227
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x fd9d33241d4a5cdf0066233bf8bdda69ea23a9f7

When the conflicts are resolved, stage and commit the changes:

git add . && git cherry-pick --continue

If you have the GitHub CLI installed:

# Push the branch to GitHub:
git push --set-upstream origin backport-14890-to-k227
# Create the PR body template
PR_BODY=$(gh pr view 14890 --json body --template 'Backport fd9d33241d4a5cdf0066233bf8bdda69ea23a9f7 from #14890{{ "\n\n---\n\n" }}{{ index . "body" }}')
# Create the PR on GitHub
echo "${PR_BODY}" | gh pr create --title 'fix: Improve docs for min and max table offsets (backport k227)' --body-file - --label 'size/M' --label 'type/docs' --label 'backport' --base k227 --milestone k227 --web

Or, if you don't have the GitHub CLI installed (we recommend you install it!):

# Push the branch to GitHub:
git push --set-upstream origin backport-14890-to-k227

# Create a pull request where the `base` branch is `k227` and the `compare`/`head` branch is `backport-14890-to-k227`.

# Remove the local backport branch
git switch main
git branch -D backport-14890-to-k227

@loki-gh-app
Copy link
Contributor

loki-gh-app bot commented Nov 14, 2024

The backport to k227 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-14890-to-k227 origin/k227
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x fd9d33241d4a5cdf0066233bf8bdda69ea23a9f7

When the conflicts are resolved, stage and commit the changes:

git add . && git cherry-pick --continue

If you have the GitHub CLI installed:

# Push the branch to GitHub:
git push --set-upstream origin backport-14890-to-k227
# Create the PR body template
PR_BODY=$(gh pr view 14890 --json body --template 'Backport fd9d33241d4a5cdf0066233bf8bdda69ea23a9f7 from #14890{{ "\n\n---\n\n" }}{{ index . "body" }}')
# Create the PR on GitHub
echo "${PR_BODY}" | gh pr create --title 'fix: Improve docs for min and max table offsets (backport k227)' --body-file - --label 'size/M' --label 'type/docs' --label 'backport' --base k227 --milestone k227 --web

Or, if you don't have the GitHub CLI installed (we recommend you install it!):

# Push the branch to GitHub:
git push --set-upstream origin backport-14890-to-k227

# Create a pull request where the `base` branch is `k227` and the `compare`/`head` branch is `backport-14890-to-k227`.

# Remove the local backport branch
git switch main
git branch -D backport-14890-to-k227

salvacorts added a commit that referenced this pull request Nov 14, 2024
Co-authored-by: Christian Haudum <christian.haudum@gmail.com>
(cherry picked from commit fd9d332)
loki-gh-app bot pushed a commit that referenced this pull request Nov 14, 2024
Co-authored-by: Christian Haudum <christian.haudum@gmail.com>
(cherry picked from commit fd9d332)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport k227 backport-failed size/M type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants