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.

Sign up for GitHub

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

Jump to bottom

v1.0: New snapshot scheduling behaviour #2120

Merged
merged 7 commits into from
Feb 2, 2023
Merged

v1.0: New snapshot scheduling behaviour #2120

merged 7 commits into from
Feb 2, 2023

Conversation

guimachiavelli
Copy link
Member

Closes #2079

@guimachiavelli guimachiavelli linked an issue Jan 17, 2023 that may be closed by this pull request
v1.0: change the behavior of --schedule-snapshot #2079
Closed
2 tasks
@guimachiavelli guimachiavelli linked an issue Jan 17, 2023 that may be closed by this pull request
v1.0: update telemetry table #2078
Closed
@guimachiavelli guimachiavelli marked this pull request as ready for review January 18, 2023 11:48
dureuill
dureuill requested changes Jan 18, 2023
View reviewed changes
Copy link
Contributor

@dureuill dureuill left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I have a few comments about the unique behavior of the flag.

learn/configuration/instance_options.md Outdated Show resolved Hide resolved
learn/configuration/instance_options.md Outdated Show resolved Hide resolved
@guimachiavelli
Copy link
Member Author

@dureuill: thanks for the review! I have tried addressing your comments in two ways:

  1. Having a longer "Default value" to reflect the different ways of using this particular option;
  2. Specifying booleans are only valid when using the config file. Seems like something user relying on automated workflows might like to know.

dureuill
dureuill approved these changes Jan 24, 2023
View reviewed changes
Copy link
Contributor

@dureuill dureuill left a comment

Choose a reason for hiding this comment

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

Thanks @guimachiavelli ! this works for me!

maryamsulemani97
maryamsulemani97 suggested changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@maryamsulemani97 maryamsulemani97 left a comment

Choose a reason for hiding this comment

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

Do you think we should mention disabling snapshots in learn/advanced/snapshots.md? It is a kind of guide, so it would be helpful to have everything snapshot-related mentioned there.

learn/advanced/snapshots.md Outdated Show resolved Hide resolved
learn/what_is_meilisearch/telemetry.md Outdated Show resolved Hide resolved
learn/configuration/instance_options.md Show resolved Hide resolved
guimachiavelli and others added 3 commits January 25, 2023 12:18
@guimachiavelli @maryamsulemani97
Apply suggestions from code review
67f590a 
Co-authored-by: Maryam <90181761+maryamsulemani97@users.noreply.github.com>
@guimachiavelli @maryamsulemani97
Update learn/what_is_meilisearch/telemetry.md
f4da775 
Co-authored-by: Maryam <90181761+maryamsulemani97@users.noreply.github.com>
@guimachiavelli
instance options: add note container
e574971
maryamsulemani97
maryamsulemani97 approved these changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@maryamsulemani97 maryamsulemani97 left a comment

Choose a reason for hiding this comment

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

🐙

@guimachiavelli guimachiavelli added this to the v1.0 milestone Jan 30, 2023
@guimachiavelli
Copy link
Member Author

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 2, 2023

Build succeeded:

@bors bors bot merged commit 079ae18 into v1.0 Feb 2, 2023
@bors bors bot deleted the v1-schedule-snapshot branch February 2, 2023 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maryamsulemani97 maryamsulemani97 maryamsulemani97 approved these changes

@dureuill dureuill dureuill approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.0
Development

Successfully merging this pull request may close these issues.

v1.0: change the behavior of --schedule-snapshot v1.0: update telemetry table
3 participants
@guimachiavelli @dureuill @maryamsulemani97