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

Regressions with conf.py and error reporting #2963

Closed
agjohnson opened this issue Jun 21, 2017 · 6 comments
Closed

Regressions with conf.py and error reporting #2963

agjohnson opened this issue Jun 21, 2017 · 6 comments

Comments

@agjohnson
Copy link
Contributor

@agjohnson agjohnson commented Jun 21, 2017

Our search should be finding conf.py files in non-standard paths, but this is not happening automatically. We should be able to find and use a conf.py at least in any first level path.

Additionally, when we aren't finding a conf.py, we were reporting this to the user. This is not happening either now, instead the user receives a message that an unknown error occured.

Ref #2962

@monvilafer
Copy link

@monvilafer monvilafer commented Aug 8, 2017

@agjohnson I would like to contribute, can you please give me a little guidance?

@agjohnson agjohnson added this to the Build stability milestone Nov 16, 2017
@humitos
Copy link
Member

@humitos humitos commented Nov 20, 2017

@monvilafer hey! I was about to start working on this issue, are you still interested on contributing?

Summarizing the steps that you will need:

  1. Setup your dev env as: http://docs.readthedocs.io/en/latest/install.html
  2. Import a project manually in your local RTD to test everything is working
  3. Start here debugging, conf_file method that tries to get the real path: https://github.com/rtfd/readthedocs.org/blob/5a4fbc93d53a876de8f19a043c64a68a9a411ecc/readthedocs/projects/models.py#L515-L537

also, the find and full_find method are interesting for this: https://github.com/rtfd/readthedocs.org/blob/5a4fbc93d53a876de8f19a043c64a68a9a411ecc/readthedocs/projects/models.py#L605-L627

@monvilafer
Copy link

@monvilafer monvilafer commented Nov 20, 2017

Yes!, I am working on it!

@humitos
Copy link
Member

@humitos humitos commented Nov 23, 2017

I think to fix this issue we will need:

  • a repository with multiples conf.py (which one would be selected?)
  • a repo without a conf.py that produce an exception and clear error description for the user
  • a repo with a conf.py file in a non-standar path (documentation/src/conf.py, for example)

Tip: I'm creating local repositories with git init and serving them with:

git daemon --verbose --export-all --base-path=.git --reuseaddr --strict-paths .git/

then, I import the repository manually and use git://localhost/ as URL.

I think this could help you to clarify what it's needed here, @monvilafer. BTW, were you able to setup the development environment?

@humitos
Copy link
Member

@humitos humitos commented Nov 27, 2017

I was able to reproduce this issue, but only in the commit mentioned in the other ticket (italia/anpr@2196e3d). latest version builds properly in my local instance but that commit raises:

[27/Nov/2017 08:44:40] readthedocs.doc_builder.environments:319[32093]: ERROR (Build) [anprlocal:multi-conf.py] Conf file not found
Traceback (most recent call last):
  File "/home/humitos/rtfd/code/readthedocs.org/readthedocs/projects/tasks.py", line 235, in run_build
    outcomes = self.build_docs()
  File "/home/humitos/rtfd/code/readthedocs.org/readthedocs/projects/tasks.py", line 430, in build_docs
    outcomes['html'] = self.build_docs_html()
  File "/home/humitos/rtfd/code/readthedocs.org/readthedocs/projects/tasks.py", line 447, in build_docs_html
    html_builder.append_conf()
  File "/home/humitos/rtfd/code/readthedocs.org/readthedocs/doc_builder/backends/sphinx.py", line 133, in append_conf
    six.reraise(ProjectImportError('Conf file not found'), None, trace)
  File "/home/humitos/rtfd/code/readthedocs.org/readthedocs/doc_builder/backends/sphinx.py", line 129, in append_conf
    outfile_path = self.project.conf_file(self.version.slug)
  File "/home/humitos/rtfd/code/readthedocs.org/readthedocs/projects/models.py", line 538, in conf_file
    u"Conf File Missing. Please make sure you have a conf.py in your project.")
ProjectImportError: Conf file not found

That produces a not very well handled exception,

[27/Nov/2017 08:44:40] readthedocs.projects.tasks:135[32093]: ERROR An unhandled exception was raised outside the build environment
Traceback (most recent call last):
  File "/home/humitos/rtfd/code/readthedocs.org/readthedocs/projects/tasks.py", line 131, in run
    self.run_build(record=record, docker=docker)
  File "/home/humitos/rtfd/code/readthedocs.org/readthedocs/projects/tasks.py", line 255, in run_build
    self.build_env.update_build(state=BUILD_STATE_FINISHED)
  File "/home/humitos/rtfd/code/readthedocs.org/readthedocs/doc_builder/environments.py", line 429, in update_build
    build_id=self.build['pk']
KeyError: 'pk'

... and the build keeps in Building forever.

@humitos
Copy link
Member

@humitos humitos commented Nov 27, 2017

The problem is this section

(Pdb++) l
527  	        files = self.find('conf.py', version)
528  	        if not files:
529  	            files = self.full_find('conf.py', version)
530  	        if len(files) == 1:
531  	            return files[0]
532  ->	        for filename in files:
533  	            if filename.find('doc', 70) != -1:
534  	                return filename
535  	        # Having this be translatable causes this odd error:
536  	        # ProjectImportError(<django.utils.functional.__proxy__ object at
537  	        # 0x1090cded0>,)
(Pdb++) 

It finaly raises the exception ProjectImportError because none of the files has doc on their path:

(Pdb++) pp files
[u'/home/humitos/rtfd/code/readthedocs.org/user_builds/anprlocal/checkouts/multi-conf.py/src/conf.py',
 u'/home/humitos/rtfd/code/readthedocs.org/user_builds/anprlocal/checkouts/multi-conf.py/src/subentro/conf.py',
 u'/home/humitos/rtfd/code/readthedocs.org/user_builds/anprlocal/checkouts/multi-conf.py/src/subentro/src/conf.py']

So, I think this logic is OK: "If there are more than one conf.py file and none of them has doc in their path, we don't know which one use" but maybe in this particular case, the error should be clearer

There are more than one conf.py file and none of them has doc in their path, we don't know which one use. Please, select the correct one under the Advanced settings tab in the project's settings.

Instead of saying

Conf File Missing. Please make sure you have a conf.py in your project.

BTW, that error is not showed either because nobody handles the ProjectImportError

humitos added a commit that referenced this issue Nov 27, 2017
`pk` exists only for Builds that are retrieved from the database, but
when it's retrieved using the API, the `id` must be used.

Using the `id` is compatible with both instances.

Related to #2963
agjohnson added a commit that referenced this issue Nov 27, 2017
`pk` exists only for Builds that are retrieved from the database, but
when it's retrieved using the API, the `id` must be used.

Using the `id` is compatible with both instances.

Related to #2963
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants