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

[mod] searx.engines.__init__: refactoring #116

Merged
merged 5 commits into from
Jun 1, 2021

Conversation

dalf
Copy link
Member

@dalf dalf commented May 29, 2021

What does this PR do?

See comments

Why is this change important?

How to test this PR locally?

Author's checklist

Related issues

searx/engines/__init__.py Outdated Show resolved Hide resolved
searx/engines/__init__.py Outdated Show resolved Hide resolved
searx/engines/__init__.py Outdated Show resolved Hide resolved
@return42
Copy link
Member

I think setting the headers to ... ja-JP

headers = {
'User-Agent': gen_useragent(),
'Accept-Language': 'ja-JP,ja;q=0.8,en-US;q=0.5,en;q=0.3', # bing needs a non-English language
}

is questionable. It comes from 9b6ffed PR-searx 2602 / there is a detailed description I do not have fully understand right now, but since I have updated my instance, sometimes (rare) I observe japan results in my german queries .

@dalf
Copy link
Member Author

dalf commented May 31, 2021

I think setting the headers to ... ja-JP is questionable.

?
This change was made to fix searx/data/engines_languages.json updates nothing more and looking at it works: searx/searx@d9dc337#diff-aea534b290d3bf70e79b052cdcf0d1b5df7cbbe92184e40bbe5b373d5699f188

@dalf dalf force-pushed the minor-refactoring-searx-engines branch from e4a3166 to 4426bc4 Compare May 31, 2021 09:53
@return42
Copy link
Member

This change was made to fix searx/data/engines_languages.json updates nothing more an

Oops .. my fail .. I have sometims (rare) japan results in my german queries, I thought it could be related to this implementation, but now I see it is totaly unrelated .. forget my comment.

@dalf dalf requested a review from return42 June 1, 2021 05:47
@dalf
Copy link
Member Author

dalf commented Jun 1, 2021

Currently, searx.engines.load_engine() exits the program when:

  • there is a missing attribute on an engine
  • there is an underscore in the name of the engine

looking at

def init(engine_settings):
if 'query_str' not in engine_settings:
raise ValueError('query_str cannot be empty')
if not engine_settings['query_str'].lower().startswith('select '):
raise ValueError('only SELECT query is supported')

it can be rewritten:

query_str = None  # instead of query_str = ""

def init(engine_settings):
    if not query_str.lower().startswith('select '):  # here `searx.engines.load_engine()` will copy the value from settings.yml to the variable
        raise ValueError('only SELECT query is supported')

ValueError won't stop the engine.


--> I suggest searx.engines.load_engine return None instead of sys.exit(1).

return42 added a commit to return42/searxng that referenced this pull request Jun 1, 2021
Loading an engine should never exit the application.  Instead of exit, return
None.

BTW: add documentation and normalize indentation (no functional change)

Suggested-by: @dalf searxng#116 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
return42 added a commit to return42/searxng that referenced this pull request Jun 1, 2021
Loading an engine should never exit the application.  Instead of exit, return
None.

BTW: add documentation and normalize indentation (no functional change)

Suggested-by: @dalf searxng#116 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
Copy link
Member

@return42 return42 left a comment

Choose a reason for hiding this comment

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

@dalf I pushed three commits on top of your branch / except 1c13bee no functional change

If you think this commits have a value just merge them along this PR into master, otherwise drop what is not needed.

Copy link
Member

@return42 return42 left a comment

Choose a reason for hiding this comment

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

Thanks! .. lets merge ..

dalf and others added 5 commits June 1, 2021 16:32
Loading an engine should not exit the application (*). Instead
of exit, return None.

(*) RuntimeError still exit the application: syntax error, etc...

BTW: add documentation and normalize indentation (no functional change)

Suggested-by: @dalf #116 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
- document namespace: searx.engines
- move docs/dev/xpath_engine.rst -> docs/src/searx.engines.xpath.rst

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
sphinx.ext.viewcode won't highlight when 'highlight_language' [1] is set to
string 'none' [2]

[1] https://www.sphinx-doc.org/en/master/usage/extensions/viewcode.html
[2] https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-highlight_language

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@dalf dalf force-pushed the minor-refactoring-searx-engines branch from 41ea099 to 15de870 Compare June 1, 2021 14:38
try:
engine = load_module(engine_module + '.py', engine_dir)
engine = load_module(engine_module + '.py', ENGINE_DIR)
except (SyntaxError, KeyboardInterrupt, SystemExit, SystemError, ImportError, RuntimeError):
logger.exception('Fatal exception in engine "{}"'.format(engine_module))
sys.exit(1)
Copy link
Member Author

@dalf dalf Jun 1, 2021

Choose a reason for hiding this comment

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

Still one sys.exit(1) here otherwise it is possible to have a syntax error in the engine code (it had happened before).

@dalf dalf merged commit ee83c99 into master Jun 1, 2021
@return42 return42 deleted the minor-refactoring-searx-engines branch June 1, 2021 15:03
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.

None yet

2 participants