Skip to content

Notifications: improve copy on error messages #11062

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Jan 26, 2024
Merged

Notifications: improve copy on error messages #11062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7dc433d
Notifications: de-duplicate/standardize concurrency limit reached
humitos Jan 24, 2024
277cbcd
Notifications: improve copy
humitos Jan 24, 2024
45be351
Remove unused `message_id`
humitos Jan 24, 2024
63b772e
Improve missing/not found `mkdocs.yml` file
humitos Jan 24, 2024
3c6e467
Improve `sphinx.configuration` file not found error message
humitos Jan 24, 2024
4c839e1
Improve copy for error messages
humitos Jan 24, 2024
55c8af0
Pass `format_values` to format the filename properly in the message
humitos Jan 24, 2024
532d65b
Apply suggestions from code review
humitos Jan 25, 2024
064b8d7
Show the type of the received value
humitos Jan 25, 2024
402392d
Better header
humitos Jan 25, 2024
eaaafe1
Lint
humitos Jan 25, 2024
3073702
Merge branch 'main' of github.com:readthedocs/readthedocs.org into hu…
humitos Jan 25, 2024
49c39b3
Render class name via custom template filter
humitos Jan 25, 2024
835d073
Make the title clearer
humitos Jan 26, 2024
9f3ea4c
Remove "\n\n\n" due to tests failing
humitos Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion readthedocs/config/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ class ConfigError(BuildUserError):
"config:python:use-system-site-packages-removed"
)
INVALID_VERSION = "config:base:invalid-version"
GENERIC_INVALID_CONFIG_KEY = "config:key:generic-invalid-config-key"
NOT_BUILD_TOOLS_OR_COMMANDS = "config:build:missing-build-tools-commands"
BUILD_JOBS_AND_COMMANDS = "config:build:jobs-and-commands"
APT_INVALID_PACKAGE_NAME_PREFIX = "config:apt:invalid-package-name-prefix"
Expand Down
55 changes: 23 additions & 32 deletions readthedocs/config/notifications.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
body=_(
textwrap.dedent(
"""
No default configuration file found at repository's root.
The required <code>readthedocs.yaml</code> configuration file was not found at repository's root.
Learn how to use this file in our <a href="https://docs.readthedocs.io/en/stable/config-file/index.html">configuration file tutorial</a>.
"""
).strip(),
),
Expand Down Expand Up @@ -66,8 +67,8 @@
textwrap.dedent(
"""
The configuration key <code>python.system_packages</code> has been deprecated and removed.
Refer to https://blog.readthedocs.com/drop-support-system-packages/ to read more
about this change and how to upgrade your config file."
<a href="https://blog.readthedocs.com/drop-support-system-packages/">Read our blog post</a>
to learn more about this change and how to upgrade your configuration file."
"""
).strip(),
),
Expand All @@ -80,8 +81,8 @@
textwrap.dedent(
"""
The configuration key <code>python.use_system_site_packages</code> has been deprecated and removed.
Refer to https://blog.readthedocs.com/drop-support-system-packages/ to read more
about this change and how to upgrade your config file."
<a href="https://blog.readthedocs.com/drop-support-system-packages/">Read our blog post</a>
to learn more about this change and how to upgrade your configuration file."
"""
).strip(),
),
Expand All @@ -99,29 +100,13 @@
),
type=ERROR,
),
Message(
id=ConfigError.GENERIC_INVALID_CONFIG_KEY,
header=_("Invalid configuration option"),
body=_(
textwrap.dedent(
"""
Invalid configuration option: <code>{{key}}</code>.

Read the Docs configuration file: <code>{{source_file}}</code>.

<code>{{error_message}}</code>
"""
).strip(),
),
type=ERROR,
),
Message(
id=ConfigError.NOT_BUILD_TOOLS_OR_COMMANDS,
header=_("Invalid configuration option: <code>build</code>"),
header=_("Missing configuration option"),
body=_(
textwrap.dedent(
"""
At least one item should be provided in "tools" or "commands".
At least one of the following configuration options is required: <code>build.tools</code> or <code>build.commands</code>.
"""
).strip(),
),
Expand Down Expand Up @@ -169,19 +154,21 @@
body=_(
textwrap.dedent(
"""
You need to install your project with pip to use <code>extra_requirements</code>.
You need to install your project with <code>python.install.method: pip</code>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is more accurate

Suggested change
You need to install your project with <code>python.install.method: pip</code>
You need to install your project's dependencies with <code>python.install.method: pip</code>

Copy link
Member Author

@humitos humitos Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure. This notification is risen when the user uses:

python:
  install:
    - method: setuptools
      extra_requirements:
        - doc

but that's incorrect. They have to install their own Python package/project with pip as follow:

python:
  install:
    - method: pip
      extra_requirements:
       - doc

Do you have a suggestion here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I had the same question here too. I feel like both are correct, really. The user is technically installing their package, but the user is also only installing their package to process the dependencies.

Your edit seems clear enough though, I don't think it's incorrect either way.

Copy link
Member Author

Choose a reason for hiding this comment

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

The user is technically installing their package, but the user is also only installing their package to process the dependencies.

I don't think this is always true. They usually install their package to be able to document its API. I will keep mine for now, but I'm open to iterate and change it.

to use <code>python.install.extra_requirements</code>.
"""
).strip(),
),
type=ERROR,
),
Message(
id=ConfigError.PIP_PATH_OR_REQUIREMENT_REQUIRED,
header=_("Invalid configuration key"),
header=_("Missing configuration key"),
body=_(
textwrap.dedent(
"""
<code>path</code> or <code>requirements</code> key is required for <code>python.install</code>.
When using <code>python.install</code>,
one of the following keys are required: <code>python.install.path</code> or <code>python.install.requirements</code>.
"""
).strip(),
),
Expand All @@ -205,7 +192,7 @@
body=_(
textwrap.dedent(
"""
You can not have <code>exclude</code> and <code>include</code> submodules at the same time.
You can not have <code>submodules.exclude</code> and <code>submodules.include</code> at the same time.
"""
).strip(),
),
Expand All @@ -230,6 +217,7 @@
textwrap.dedent(
"""
Error while parsing <code>{{filename}}</code>.
Make sure your configuration file doesn't have any errors.

{{error_message}}
"""
Expand Down Expand Up @@ -261,7 +249,8 @@
textwrap.dedent(
"""
Config validation error in <code>{{key}}</code>.
Expected one of (0, 1, true, false), got <code>{{value}}</code>.
Expected one of <code>[0, 1, true, false]</code>, got type <code>{{value|to_class_name}}</code> (<code>{{value}}</code>).
Make sure the type of the value is not a string.
"""
).strip(),
),
Expand All @@ -274,7 +263,9 @@
textwrap.dedent(
"""
Config validation error in <code>{{key}}</code>.
Expected one of ({{choices}}), got <code>{{value}}</code>.
Expected one of ({{choices}}), got type <code>{{value|to_class_name}}</code> (<code>{{value}}</code>).
Double check the type of the value.
A string may be required (e.g. <code>"3.10"</code> insted of <code>3.10</code>)
"""
).strip(),
),
Expand All @@ -287,7 +278,7 @@
textwrap.dedent(
"""
Config validation error in <code>{{key}}</code>.
Expected a dictionary, got <code>{{value}}</code>.
Expected a dictionary, got type <code>{{value|to_class_name}}</code> (<code>{{value}}</code>).
"""
).strip(),
),
Expand Down Expand Up @@ -326,7 +317,7 @@
textwrap.dedent(
"""
Config validation error in <code>{{key}}</code>.
Expected a string, got <code>{{value}}</code>.
Expected a string, got type <code>{{value|to_class_name}}</code> (<code>{{value}}</code>).
"""
).strip(),
),
Expand All @@ -339,7 +330,7 @@
textwrap.dedent(
"""
Config validation error in <code>{{key}}</code>.
Expected a list, got <code>{{value}}</code>.
Expected a list, got type <code>{{value|to_class_name}}</code> (<code>{{value}}</code>).
"""
).strip(),
),
Expand Down
7 changes: 4 additions & 3 deletions readthedocs/doc_builder/backends/mkdocs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,10 @@ def append_conf(self):
value = []
if not isinstance(value, list):
raise MkDocsYAMLParseError(
MkDocsYAMLParseError.INVALID_EXTRA_CONFIG.format(
config=config,
),
message_id=MkDocsYAMLParseError.INVALID_EXTRA_CONFIG,
format_values={
"extra_config": config,
},
)
# Add the static file only if isn't already in the list.
value.extend([extra for extra in extras if extra not in value])
Expand Down
5 changes: 4 additions & 1 deletion readthedocs/doc_builder/backends/sphinx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,10 @@ def append_conf(self):

if not os.path.exists(self.config_file):
raise UserFileNotFound(
UserFileNotFound.FILE_NOT_FOUND.format(self.config_file)
message_id=UserFileNotFound.FILE_NOT_FOUND,
format_values={
"filename": os.path.relpath(self.config_file, self.project_path),
},
)

# Allow symlinks, but only the ones that resolve inside the base directory.
Expand Down
1 change: 0 additions & 1 deletion readthedocs/doc_builder/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ class BuildAppError(BuildBaseException):
class BuildUserError(BuildBaseException):
GENERIC = "build:user:generic"
SKIPPED_EXIT_CODE_183 = "build:user:exit-code-183"
MAX_CONCURRENCY = "build:user:max-concurrency"

BUILD_COMMANDS_WITHOUT_OUTPUT = "build:user:output:no-html"
BUILD_OUTPUT_IS_NOT_A_DIRECTORY = "build:user:output:is-no-a-directory"
Expand Down
14 changes: 8 additions & 6 deletions readthedocs/doc_builder/python_environments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,10 @@ def _append_core_requirements(self):
)
if not inputfile:
raise UserFileNotFound(
UserFileNotFound.FILE_NOT_FOUND.format(
self.config.conda.environment
)
message_id=UserFileNotFound.FILE_NOT_FOUND,
format_values={
"filename": self.config.conda.environment,
},
)
environment = parse_yaml(inputfile)
except IOError:
Expand Down Expand Up @@ -334,9 +335,10 @@ def _append_core_requirements(self):
)
if not outputfile:
raise UserFileNotFound(
UserFileNotFound.FILE_NOT_FOUND.format(
self.config.conda.environment
)
message_id=UserFileNotFound.FILE_NOT_FOUND,
format_values={
"filename": self.config.conda.environment,
},
)
yaml.safe_dump(environment, outputfile)
except IOError:
Expand Down
Loading