search

elastic / connectors-ruby

Official Connector Clients for Elastic Elasticsearch, Enterprise Search, App Search and Workplace Search
https://www.elastic.co/guide/en/enterprise-search/master/index.html
Other
9 stars 17 forks source link

Modify the structure of scheduling #553

Closed wangch079 closed 1 year ago

wangch079 commented 1 year ago

Part of https://github.com/elastic/enterprise-search-team/issues/4589

Part of https://github.com/elastic/enterprise-search-team/issues/4628

This PR modifies the structure of scheduling, to accommodate different types of scheduling.

Checklists

Pre-Review Checklist

wangch079 commented 1 year ago

LGTM - might be worth to add logic that also checks if any other type of schedule is there and shows error/warning that schedule other than full is not supported

We don't have this kind of validation in python either. We can't prevent users from adding something funny as we don't enforce strict mapping, but I think it's fine because we only care about full, incremental and permissions.

artem-shelkovnikov commented 1 year ago

We don't have this kind of validation in python either. We can't prevent users from adding something funny as we don't enforce strict mapping, but I think it's fine because we only care about full, incremental and permissions.

What I mean is that Ruby won't support even incremental/permissions, so if someone decides to use MongoDB in Python and then switches to Ruby it'll be a legit case that Ruby does not support and might be worth to communicate to user.

wangch079 commented 1 year ago

so if someone decides to use MongoDB in Python and then switches to Ruby it'll be a legit case that Ruby does not support and might be worth to communicate to user.

🤔 Mmm... is it even a valid use case? we are not supporting Ruby connectors anymore. We made this PR to support Crawlers, not connectors.

artem-shelkovnikov commented 1 year ago

🤔 Mmm... is it even a valid use case? we are not supporting Ruby connectors anymore. We made this PR to support Crawlers, not connectors.

Not sure, just wanted to double check with you how you feel about it

wangch079 commented 1 year ago

Not sure, just wanted to double check with you how you feel about it

I think the only reason we make some changes to connectors-ruby, is either 1) there's severe vulnerability or 2) there's breaking changes to crawler(I'm not even sure about point 1).

If the scheduling structure change doesn't affect crawler, I don't think I will make this PR, even though it means connectors-ruby won't work with Elasticsearch and Enterprise Search 8.9. Because we are not releasing any new versions (surprisingly to see we still have 8.8 branch)