-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Config file: support for multiple config files in a project #8811
Comments
I don't understand option B: where would |
That's an option on the project admin UI. |
Gotcha, I vote for B as well |
I think this is a good case to come back to "project level configs Vs. version level configs" discussion and try to solve the problem in a more general way. Make a decision, plan the path going forward, and create the basic structure to keep adding "project level configs" in a clean way in the future. We have discussed this in #6046, #4221, #5478, #8678 (there are more issues) and we also have some cards in our Trello.
I think this new field proposed is probably the first step towards the general solution, but I'd like to see a plan involving this thinking a little more. For example:
I'm not saying this solution is good, but I think we need to think something among these lines to solve this problem and start building the new features on top of the decision we make here. |
Not sure if that solves the problem, or if I understand your solution. If the option is defined on a file on the repo, the user can't have different values for the same repo on the same commit/branch (which is the problem on a monorepo). |
To have different configs/settings for the same branch you need to have two different Besides, I'm saying that we need another config file path for "project-level configs" where the user can set up as I'm not proposing a final solution. However, I'm saying that we need to figure out how to do this in a clean way. |
That is a great point @humitos 💯 To back up just a bit: to support monorepos we can solve this purely at the Sphinx level, configured via env vars, and that does at least work. We could still decide this isn't a good fit for our application and solve this outside, via Sphinx extension. I would be okay if we ended here, because this isn't a widely requested feature at the end of the day. Okay, so now if we do want to support this natively, I came to the same conclusion as Manuel. We can tell users to use a singular python requirements file in a monorepo, but users probably would still want per-project configuration of:
When we get to redirects, we need per-project and per-version configuration. So we need two files to support this. But once we also factor in monorepo support, we're talking about multiple sets of these configuration files -- as noted above. We have a few potential options:
Namespacing isn't as awful as it sounds. Consider something like this: version: 2
formats: all
sphinx:
configuration: docs/conf.py
overrides:
"api-docs":
sphinx:
configuration: docs/api/conf.py
search: {}
"site-docs":
sphinx:
configuration: docs/site/conf.py
"dev-docs":
sphinx:
configuration: docs/dev/conf.py
search:
ranking:
# Deprecated content
api/v1.html: -1
config-file/v1.html: -1
# Useful content, but not something we want most users finding
changelog.html: -6
Dynamic configuration is maybe a 🤯 solution, but it can also solve future configuration problems and provides a lot of flexibility to users. We're handing out foot guns with this, but you're already advanced usage if you want multiple project support. Looking at some prior art, CircleCI has a feature for monorepo usage, called dynamic config: It's a script that generates the configuration file used in the build. It's a heavy solution, and quite weird, but could be inspiration. But, that got me thinking, what about jinja template YAML? version: 2
formats: all
# Basic templating can be used for variables
sphinx:
configuration: docs/{{ readthedocs.project_slug }}/conf.py
# But we even have includes on our config
{% if readthedocs.project_slug == 'foo' %}
{% include '.readthedocs/foo.yaml' %}
{% endif %} Namespaces/overrides seems like a sweet spot for advanced usage and maintainability/support on our side. I could see some incredibly cool things coming from template support though, but it's more of an expert level feature given you need to know jinja syntax too. Anyways, I agree with Manuel, now is a good time to think ahead to a global configuration file. It would help to consider how we might use a global configuration file and how this might shape up that spec -- though we can simmer on an actionable plan for both redirects and global configuration. |
I think explicit is better, but also having two config files would be confusing (I'm talking about the idea to have one config file for global settings and another one for per-version settings). If we have two files for global and per-version settings, we still need to choose from what branch/version we read that file, if we go with reading global options from the default version or latest, we can just use the same config file, adding the settings under the "global" key, something like: version: 2
formats: all
sphinx:
configuration: docs/conf.py
search:
ranking:
# Deprecated content
api/v1.html: -1
config-file/v1.html: -1
global-settings:
redirects: ? For the monorepo support, I still think having a setting on the admin is a good place for it instead of a file (so we don't need to guess from what file to read this setting that will tell us from what other file read the real settings), and will also give more flexibility about where to put the file, each config under each subdirectory (but we need to be explicit about relative and absolute paths).
This is mainly to share the same config between projects, but I think most of the time there is little to share, like search the paths on each project will be different, and will also bring confusion about what overrides what (are values merged or replaced?). |
This is going to be a problem regardless of the number of files. There needs to be a single, canonical file + branch that is responsible for project-level settings, so this is still going to be confusing UX.
Hrm yeah, this would be worth discussing more. Given the point above, which is less confusing: a single file with a globals key, or separate configuration file? I'm not sure there is a really strong benefit either way, so would maybe lean to a singular file. We'd probably want something other than At that point, the maintainer's UX around project-level settings is probably similar whether they are authoring a special file or a special key in our normal file. I do like a configuration file setting for the file name more than I do a configuration postfix -- the postfix idea is much harder to communicate.
Yeah, I don't think there is a way to do this without a UI setting. Even in my namespace example above, there would need to be a UI setting for the namespace to use.
Yeah, it's only arguably better UX if we have a single config file. Users most likely are just going to change the Sphinx dir setting between copies. But, if we have multiple sets of configuration files, I would do namespacing before supporting a |
Also, noted on a call today, a configurable readthedocs.yaml path makes support harder. This was seeming like an okay option otherwise, though mostly for the use case of multiple sphinx projects. The conversation here will still be applicable to redirect and project-level configuration in our config file, but for supporting multiple projects and shared documentation sources, we're going to start with an extension instead. |
Another data point: due to #8995, PyPy (which uses the same repo to generate https://pypy.readthedocs.io/en/latest/ * and https://rpython.readthedocs.io/en/latest/) needs to start using a config file. So we need the ability to specify multiple config files.
|
This seems like the obvious path forward, similar to what we're doing with conf.py historically. I think we really need to find a way to make this configurable, and putting an option in the DB seems like the obvious place? I think it's worth any added complexity to have a good answer that's simple to understand and implement, for users with a monorepo or similar. |
netlify has a "base directory" option for cases like this https://docs.netlify.com/configure-builds/overview/#basic-build-settings (instead of having to specify the path to a config file, the user will specify a path to a directory). |
I think Also, one of the issues around "make support harder" mentioned by @agjohnson is that we (and users) will need to check multiple places to find our which is the correct configuration file read by that project in particular. By default, everybody thinks it will be Another "feature" that we will lose by adding more configs to the database is that deleting the project and re-importing it may not work --because configs saved into the database will be lost. I'm not saying that adding a new config into the database is bad, but we've been avoiding this on purpose for these reasons and others. So, I think we should consider them and think more deeply if it's a good idea and if we are fine covering the extra complexity needed (on the daily basis support) before moving forward with it --even if "it's just a new field in the database" looks simple to implement. Finally, the "namespace's idea" from @agjohnson (#8811 (comment)) doesn't look bad to me and we could not depend on an extra db field (as he suggested: |
This issue was renamed, but I think the old title is more accurate to what we have discussed initially (support for multiple config files in one project), and I think this feature should still be considered, it's easier to share the same repo, but hard to have different requirements (well, this is possible with a custom script using build.jobs), but other settings like search can't be separated between projects. I'm opening a new issue to discuss the project level options in the config file at #9188 |
Just another note, with |
Thank you for the hint @ericholscher |
You can create a script that maps each project (using the You can also make use of this extension https://github.com/readthedocs/sphinx-multiproject. |
great, thank you very much :) |
I really like your first point @humitos, bringing in And I'm not even thinking about building new awesome features with different configurations for projects and versions, my trail of thought is from reproducible builds. I.e. if a single database field outside of Git history decides a critical build aspect, then we're moving away from reproducible builds. I don't think that a "base directory" or "config file namespaces" fundamentally changes this. |
re: the implementation from #10001 This is the second idea I put on the original issue. The main downside would be: When building an old version, we will try to use the current value of the config file name, and probably those versions were using the old name. We could solve this by introducing the same setting at the version level, if set to But there is a workaround, changing the project-level value before building the "problematic" version, so maybe we can decide about introducing the per-version config later. |
I'd be interested in hearing your comment to my attempt of a practical TODO for #10001 here: #10001 (comment) |
I think we don't need to track the modified date or have lots of rules if we just allow users to set the config file per version explicitly, then we just have a simple check |
I think that a I'm not saying that the UI would be implemented in version 1 MVP... but storing that date is "free", so I'd just do that and see what happens later. It'd keep this data, it might become important. It means that we have a bit more freedom to come up with a relaxed analysis of what "reproducibility" means now and then return to the subject later. |
Changing the value of the project level config will have effect only over versions that don't have an explicit value set (version level config has priority over project config level), the date isn't relevant in that change. If you mean, something like "bulk" update, that's probably better done using the API, like the user can query all versions that don't have an explicit config file value set, and set it to the current value before changing the new value at the project level. And if we want this information for analytics or similar, we already track when a project changes with our Historical models
Isn't that easy to query, but the information is there. |
@stsewd makes sense that the data can be fetched in historical records, just want to make the option explicitly available for later UI interactions.
I would leave that up to the user. They are in charge of their own git release branches and can choose to change the config file layout here as well. Edit: But I'd also defer this entire part of the design until later, just want to make sure that we are free to implement whatever is asked for. |
I agree with @stsewd, we want this implementation to be super simple, and not over-engineered. If we find user requirements that need some complexity, we can discuss, but I really want it to be explicit. @stsewd can you explain (or link in the above massive thread) the idea of doing it per version instead of per-project? I got sucked into another large thread around redirects in the config file, so I think refreshing the debate here makes sense. In particular it feels like we have a few options in my mind:
My primary goal here is to unblock users who are hitting this problem currently, and get a bit more data on what works. I think a path at the project level solves this, but does have downsides that we know historically around applying to all versions at once. I'm fine with Version-specific, as that's more flexible/explicit, but seems messy to maintain, but at least explicit. To address the above points around debugging -- I think the new build pages makes this a lot more explicit with our debug view that could show an explicit |
I think most of the discussion about project level vs version level was mostly related to #9188, rather than having a path to the config file itself.
There was also the idea about namespaces that Anthony mentioned in #8811 (comment). Which is kind of similar to 1), but using different files instead of just one. |
I think it's a bigger challenge to come up with a different approach than the one that's already sponsored by community, that's why I really like #10001. I can see it work, so I'm pitching in safeguards to ensure that different design choices can be left open. I don't think that's over-engineering @ericholscher. I think over-engineering is when you create complexity that's only useful in a hypothetical case. Storing a bit of extra data isn't complex. If there are things in my list of extra small TODOs to add that are in fact complex, we can trim them out 👍 |
It depends :) Where do we store that data, how long do we keep it, do we show it to users, is it lies in some cases, etc? But I don't want to get too deep into specific implementation trade offs... Reframing the debate on our short-term goalsBut I really want to focus the goal that I'm trying to achieve:
It's goal 2 where we keep getting stuck :) I think just adding a feature-flagged project-level config solves this for now, without adding a bunch of additional work around versions, changes, updates, etc. "It's project-level, and we know how that breaks, if that works for you, here it is, just email us to enable it". So I think the more actionable question I have is: "What downside risk is there to enable project-level Or a @benjaoming put it:
💯 -- let's do that, but not ship it to everyone, to give ourselves an option to change course if needed. Instead of engineering safeguards, let's just gate it behind email -- it's the best safeguard, and gives us information in which to further our design discussion with real use cases, instead of theoretical ideas we have. |
This discussion went pretty deep and complex in multiple directions. I'll try to concise and expose my thoughts about the 2 different alternatives that I consider valid and doable: New field into the databaseI think the easiest way to solve this problem is by adding a new Overrides at the same
|
We had a call around this, and decided to move forward with the approach in #10001. High level:
Requirements:
|
A quick update on why this issue is now closed and fixed: The proposal from @ewdurbin in #10001 was adopted and will premiere in the next deployment. If there's any feedback, please don't hesitate to write here or open a new issue 👍 Thanks for all the thoughts, initiatives and good riddance to address this challenging new feature ❤️ |
Btw. it might be interesting to open up any follow-up issues to the approach, but maybe that should anticipate a bit more usage and user feedback, thoughts @ericholscher ? |
FTR, here are the docs https://docs.readthedocs.io/en/latest/guides/setup/monorepo.html 🥳 |
I just saw this option in the web UI for the first time 👍 |
Thanks @CagtayFabry ❤️ (cross-posting from #10001) There's a little public example of the new feature available here: https://github.com/readthedocs-examples/example-monorepo |
Currently, the path of the config file can't be changed, it always is
.readthedocs.yaml
and should be at the root of the repo.But there are users with monorepos that have several projects and their docs in one repo, so using a config file isn't something they can do, they will need to use the settings from the UI, which limit the options they can set and makes them hard to track.
By being able to change the path of the config file per-project, those users would be able to have a better experience. So, the question is how to expose this option, I have two suggestions:
RTD_CONFIG=subproject/readthedocs.yaml
project.config_path
.So, I'm more inclined to option B, do you have another idea of how to implement it?
The text was updated successfully, but these errors were encountered: