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

Add "NOT PRODUCTION READY" disclaimer in syncing with meilisync chapters #2926

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

vaddenz
Copy link
Contributor

@vaddenz vaddenz commented Aug 5, 2024

Pull Request

What does this PR do?

After spending some time engaing with meilisync, it turns out that this project is still in a state of flux, lacking comprehensive testing and readiness for deployment in a live environment which is NOT suitable in the official document. I think it is necessary to address potential issues so that users could pay attention to this and make their own decisions.

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Added "NOT PRODUCTION READY" disclaimer
Add "NOT PRODUCTION READY" disclaimer
guimachiavelli
guimachiavelli previously approved these changes Aug 5, 2024
Copy link
Member

@guimachiavelli guimachiavelli left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, @vaddenz.

It looks good and I'm preemptively approving it. I'll leave the final call to @Strift, though.

That said, we've been getting more and more reports from users having problems with meilisync. I'm starting to wonder whether it wouldn't be better to remove this guide, since people shouldn't need to wonder whether a tool featured in our docs is safe to use.

Strift
Strift previously approved these changes Aug 5, 2024
Copy link
Contributor

@Strift Strift left a comment

Choose a reason for hiding this comment

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

Hi @vaddenz, and thank you for your contribution.

I'm sorry to hear your feedback about Meilisync, and support Gui's opinion here. We might have to reconsider recommending Meilisync altogether in the future.

We opened a discussion on the topic, your feedback is more than welcome: RFC: database sync tool

Thanks,

Remove the blank line
@vaddenz vaddenz dismissed stale reviews from Strift and guimachiavelli via 89785e9 August 5, 2024 13:55
@vaddenz
Copy link
Contributor Author

vaddenz commented Aug 5, 2024

Hi guys, thanks for the quick review. I appreciate you and meilisync's authors brilliant hard work and agree that removing it is better.

@guimachiavelli
Copy link
Member

@vaddenz, if I may ask for another bit of your time: could you describe what issues you had when using meilisync? We're currently collecting feedback from meilisync users, which may help our support team field some of the questions coming in.

@vaddenz
Copy link
Contributor Author

vaddenz commented Aug 7, 2024

@vaddenz, if I may ask for another bit of your time: could you describe what issues you had when using meilisync? We're currently collecting feedback from meilisync users, which may help our support team field some of the questions coming in.

Hi @guimachiavelli , here are some issues I've experienced:

  1. Usability: Following the project's configuration guide, the project was unable to start and throwed some exceptions. Here is a related issue: TypeError: 'async for' requires an object with __aiter__ method, got list long2ice/meilisync#94
  2. Reliability: Seems that meilisync was unable to capture some changes made in the DB
  3. Poor maintenance

As for the sync tool, I'd suggest that MeiliSearch consider officially integrating some common utilized projects, e.g. Canal, Debezium etc

@guimachiavelli
Copy link
Member

Brilliant, thanks for the feedback, @vaddenz! I'll pass this along to support and to our product managers, as it will really help them both managing current support requests and solving the issue more definitively in the future!

thanks again!

@guimachiavelli guimachiavelli merged commit b998418 into meilisearch:main Aug 7, 2024
1 check passed
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