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

feat: add auto_indexing param to register decorator function #305

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

KarimTayie
Copy link

@KarimTayie KarimTayie commented Jan 13, 2021

Q A
Bug fix? no
New feature? yes
BC breaks? no
Related Issue closes #277 #276
Need Doc update no

Describe your change

  • Added auto_indexing parameter to register decorator function with None as a default value
    for backward compatibility.

  • if a boolean value has been passed to register decorator or register function for auto_indexing parameter
    prioritize it over the value from the settings.

What problem is this fixing?

  • With this change, I can enable/disable auto_indexing on any specific sub-class of AlgoliaIndex with register decorator.

  • Now I can make the auto_indexing setting=True but when I call register function or decorator with auto_indexing=False
    it will take it and prioritize it over the setting value.

Copy link
Contributor

@tkrugg tkrugg left a comment

Choose a reason for hiding this comment

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

Hi @KarimTayie, thank you for your contribution!

I think your PR might be addressing #277, can you confirm?
It's also addressing: #276.

The change looks great. Please would you be able to add some tests in tests/test_decorators.py and tests/test_engine.py to verify this is working as expected?

@tkrugg tkrugg requested review from clemfromspace, chloelbn and tkrugg and removed request for tkrugg March 9, 2021 14:29
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.

Suggestion: accept auto_indexing=False in registration decorator
2 participants