Skip to content

DATAUP-748 use locale en_US & other cleanup#114

Merged
ialarmedalien merged 1 commit intodevelopfrom
DATAUP-748-test-locale
Apr 30, 2022
Merged

DATAUP-748 use locale en_US & other cleanup#114
ialarmedalien merged 1 commit intodevelopfrom
DATAUP-748-test-locale

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Apr 29, 2022

Change the locale of the ci::icu_tokenize analyzer to en_US and do some comprehensive-ish tests (a subset of the 10-day tests) to ensure that the index and AQL tokenizer still agree on word boundaries and such.

Also removed the generic/fulltext_search.yaml which I wrote so I know it's not used anywhere because it has performance issues.

  • I updated the README.md docs to reflect this change.

For changes to the codebase:

  • I have written tests to cover this change.
  • This is not a breaking API change OR
  • This is a breaking API change and I have incremented the API version and added a summary to CHANGELOG.md.

import os
from uuid import uuid4

from relation_engine_client import REClient
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This was confusing since relation_engine_client is a directory with an __init__.py that defined __all__ with main.REClient

Comment thread client_src/test/test_integration.py Outdated
Comment on lines +53 to +56
@unittest.skip(
"Insofar inexplicable requests.exceptions.ConnectionError. "
"Failure mode for empty query not imperative"
)

This comment was marked as outdated.

if scheme in self.handlers:
result = self.handlers[scheme](uri)
elif scheme in [u"http", u"https"]:
elif scheme in ["http", "https"]:
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Autoformatters

@@ -1,94 +0,0 @@
# Should be REVISED or DEPRECATED.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I wrote this and I know it's not being used because of performance issues so axed the cruft

)


class TestFulltextSearchStoredQuery(unittest.TestCase):
Copy link
Copy Markdown
Author

@ghost ghost Apr 29, 2022

Choose a reason for hiding this comment

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

Removed tests pertaining to now axed spec/stored_queries/generic/fulltext_search.yaml

@ghost ghost marked this pull request as ready for review April 29, 2022 20:57
@ghost ghost requested a review from ialarmedalien as a code owner April 29, 2022 20:57
Copy link
Copy Markdown
Collaborator

@ialarmedalien ialarmedalien left a comment

Choose a reason for hiding this comment

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

Can you do a little more investigation into the admin query failure? Better still, remove that change from this PR (the rest is fine) and then do some investigation in another PR.

@ghost ghost force-pushed the DATAUP-748-test-locale branch from 2da9bd5 to dee3ce3 Compare April 29, 2022 22:11
@ghost ghost force-pushed the DATAUP-748-test-locale branch from dee3ce3 to bc5c32d Compare April 29, 2022 22:16
Comment thread .dockerignore
docker-compose*

# Temp files
tmp/
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

These are generated by the comprehensive query testing, or any output files can go here

Comment thread Makefile

reset:
docker-compose --rmi all -v
docker-compose build
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Forgot "down", and the "build" is redundant since the other make targets do it

@ghost
Copy link
Copy Markdown
Author

ghost commented Apr 29, 2022

Yep, was planning on that

@ghost ghost requested a review from ialarmedalien April 30, 2022 00:05
Copy link
Copy Markdown
Collaborator

@ialarmedalien ialarmedalien left a comment

Choose a reason for hiding this comment

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

LGTM

@ialarmedalien ialarmedalien merged commit e556289 into develop Apr 30, 2022
@ialarmedalien ialarmedalien deleted the DATAUP-748-test-locale branch April 30, 2022 01:12
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.

1 participant