Summary

Pass the RBTools configuration to the HTTP/API layer.

Review Request #14221 — Created Nov. 2, 2024 and updated Nov. 5, 2024, 6:17 p.m.

Information

Owner

chipx86

Repository

RBTools

Branch

release-5.x

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

We currently pass a lot of options all the way from RBClient to the
transport to ReviewBoardServer, and this list has continued to grow
over time. We will soon be needing another option, and instead of
continuing to add dedicated options, we're now passing the loaded
configuration instead.

RBToolsConfig was added in RBTools 5, and consolidates all of our
loaded options. This is easier to populate/load and pass around.
RBClient, SyncTransport, and ReviewBoardServer now all take this
as arguments.

If this is not provided, ReviewBoardServer will load a configuration.
This avoids breaking API compatibility with any custom scripts or hooks.
The loaded configuration will always be explicitly provided when being
run from a command.

Testing Done

Unit tests passed.

Successfully posted this change for review.

Made use of the configuration in an in-progress change.

Commits

Summary	ID
Pass the RBTools configuration to the HTTP/API layer. We currently pass a lot of options all the way from `RBClient` to the transport to `ReviewBoardServer`, and this list has continued to grow over time. We will soon be needing another option, and instead of continuing to add dedicated options, we're now passing the loaded configuration instead. `RBToolsConfig` was added in RBTools 5, and consolidates all of our loaded options. This is easier to populate/load and pass around. `RBClient`, `SyncTransport`, and `ReviewBoardServer` now all take this as arguments. If this is not provided, `ReviewBoardServer` will load a configuration. This avoids breaking API compatibility with any custom scripts or hooks. The loaded configuration will always be explicitly provided when being run from a command.	c11b0c7632682b0a059c0105f5e687148956852a

Summary

Pass the RBTools configuration to the HTTP/API layer.

We currently pass a lot of options all the way from `RBClient` to the transport to `ReviewBoardServer`, and this list has continued to grow over time. We will soon be needing another option, and instead of continuing to add dedicated options, we're now passing the loaded configuration instead. `RBToolsConfig` was added in RBTools 5, and consolidates all of our loaded options. This is easier to populate/load and pass around. `RBClient`, `SyncTransport`, and `ReviewBoardServer` now all take this as arguments. If this is not provided, `ReviewBoardServer` will load a configuration. This avoids breaking API compatibility with any custom scripts or hooks. The loaded configuration will always be explicitly provided when being run from a command.

c11b0c7632682b0a059c0105f5e687148956852a

Issues

Description	From	Last Updated
Now that we have this config option, should we deprecate all of the individual options?	maubin	Nov. 5, 2024, 6:16 p.m.
If there's a value set in config and a different value set as an individual option, which value should be …	maubin	Nov. 5, 2024, 6:17 p.m.

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

The issue has been dropped. Show all issues

Now that we have this config option, should we deprecate all of the individual options?

david Nov. 4, 2024, 9:13 a.m.

Unfortunately right now the things in options take precendence over things in config (and any options which have config-based defaults will get populated as the default for the option). I want to change this so we merge options into config instead of vice-versa, but that's a bigger project.

maubin Nov. 5, 2024, 2:57 p.m.

Oh I see. We should add some docs or comments to guide callers on what to do with options vs the config.

The issue has been dropped. Show all issues

If there's a value set in config and a different value set as an individual option, which value should be used?

chipx86 Nov. 5, 2024, 5:07 p.m.

Right now there's nothing used in config outside of this one key. We don't treat it special or default any arguments based off of this today. I think for this release, we're going to just make this an "etc" object, prioritizing the other variables. Eventually I want to give the API and transport machinery a hard look, at which point we'll address the rest of this.

maubin Nov. 5, 2024, 6:17 p.m.
```
Gotcha, sounds good to me.
```

Ship it!

```
Ship It!
```