-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Remove doctype from search #5121
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
Conversation
I just tested this with sphinx and mkdocs, everything built normally. |
self.conf_dir(version), | ||
'_build', 'json' | ||
) | ||
return json_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, this is called from process_all_json_files
And is used in os.walk
, if the path doesn't exists (mkdocs for example), it doesn't raise an error, it just returns an empty iterator.
@@ -1350,7 +1344,8 @@ def sync_callback(_, version_pk, commit, *args, **kwargs): | |||
The first argument is the result from previous tasks, which we discard. | |||
""" | |||
fileify(version_pk, commit=commit) | |||
update_search(version_pk, commit=commit) | |||
if kwargs.get('search'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be needed bc the search code was updated too, but just in case.
I will try and test this locally, but it looks good at first look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed up the search passing arg, and things look good locally 👍
readthedocs/projects/tasks.py
Outdated
@@ -741,6 +741,7 @@ def update_app_instances( | |||
callback=sync_callback.s( | |||
version_pk=self.version.pk, | |||
commit=self.build['commit'], | |||
kwargs={'search': search}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ending up:
> /Users/eric/projects/readthedocs.org/readthedocs/projects/tasks.py(1395)sync_callback()
1394 import ipdb; ipdb.set_trace()
-> 1395 if kwargs.get('search'):
1396 update_search(version_pk, commit=commit)
ipdb> kwargs
{'kwargs': {'search': True}}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing in a push.
Woops. Also ended up pushing my #5150 since I had to have it to test the branch. After that is merged, this should look normal again :) |
23e4e61
to
7417e7c
Compare
45bf9f8
to
9d9947a
Compare
Mergable when tests pass 👍 |
I still want to test this, I'm going to set up ES later.
Related #4896 (review)