Adds config files for the API

[marcosfrenkel]

The server looks for a file called config.toml wherever is running, and if it cannot find it there it looks for a file specified in the environment variable API_CONFIG_PATH

Config files should look something like this:

# Router configuration
router_name = "router1"
router_address = "xx.xx.xx.xx"
router_port = 5555

# Timetagger configuration (provider_name, instrument_name)
timetagger = ["provider", "ins"]

# Bell state configuration (default: Phi_plus)
bell_state = 0

# CHSH experiment settings
[chsh_settings]
hwp = ["provider", "ins"]
request_hwp = ["provider", "ins"]

# CHSH measurement configuration
[chsh_settings.measurement_config]
integration_time_s= 5
binwidth = 500
channel1 = 1
channel2 = 2
dark_count = 0

# QKD experiment settings
[qkd_settings]
hwp = ["provider", "ins"]
request_hwp = ["provider", "ins"]
bitstring_length = 4
discriminating_threshold = 10

# QKD measurement configuration
[qkd_settings.measurement_config]
integration_time_s = 5
binwidth = 500
channel1 = 1
channel2 = 2
dark_count = 0

[Benjamin-Nussbaum]

The only comment I have is to consider using pydantic_settings, which should handle a lot of the boilerplate here. https://docs.pydantic.dev/latest/concepts/pydantic_settings/

[marcosfrenkel]

Not entirely sure why it says I dismissed your review but I have updated it to use BaseSettings now. If it looks good we can merge @Benjamin-Nussbaum

[Benjamin-Nussbaum]

I don't think this is necessary since pydantic-settings has dotenv support - see https://docs.pydantic.dev/latest/concepts/pydantic_settings/#dotenv-env-support

[marcosfrenkel]

Removed

[Benjamin-Nussbaum]

Consider using @lru_cache from functools https://fastapi.tiangolo.com/advanced/settings/?h=settings#creating-the-settings-only-once-with-lru_cache

[marcosfrenkel]

Since we are simply importing the settings object and using it there, the get_settings() only gets called once already. I don't think most people know about that function so I don't want to add more complexity for basically no new functionality

[Benjamin-Nussbaum]

If we have dependencies with other endpoints in addition to the config, it might add to the complexity of why one is a (Annotated) Depends dpendency and another is directly imported. If we want to directly import, the get_settings() function itself is unnecessary; we can just instantiate the Settings object at the module level directly. But it makes more sense to me to stick to the FastAPI standard using Depends, in which case I also recommend using @lru_cache.

[Benjamin-Nussbaum]

pydantic is a dependency of pydantic-settings (it should be an option, but that's not how they set it up). Anyway, the separate pydantic dependency here is not necessary with pydantic-settings.

[marcosfrenkel]

That was the case with the first version of pydantic but they split it up for the 2.0 version the info box in this section

[Benjamin-Nussbaum]

https://github.com/pydantic/pydantic-settings/blob/main/pyproject.toml#L43

[Benjamin-Nussbaum]

Maybe this example should be in ./configs/? Or perhaps a new ./examples/ directory?

[marcosfrenkel]

done, I have also added the messaging config example as well

[Benjamin-Nussbaum]

https://github.com/pydantic/pydantic-settings/blob/main/pyproject.toml#L43

[Benjamin-Nussbaum]

Suggested change

"pydantic>=2.11.7",

[Benjamin-Nussbaum]

If we have dependencies with other endpoints in addition to the config, it might add to the complexity of why one is a (Annotated) Depends dpendency and another is directly imported. If we want to directly import, the get_settings() function itself is unnecessary; we can just instantiate the Settings object at the module level directly. But it makes more sense to me to stick to the FastAPI standard using Depends, in which case I also recommend using @lru_cache.

[marcosfrenkel]

ok I think that was all the suggestions. If it looks good can we merge @Benjamin-Nussbaum ?

[Benjamin-Nussbaum]

@lru_cache should be on get_settings(), not the Settings object itself. https://fastapi.tiangolo.com/advanced/settings/?h=settings#creating-the-settings-only-once-with-lru_cache

Otherwise looks good

[Benjamin-Nussbaum]

Suggested change

	def get_settings() -> Settings:
	@lru_cache
	def get_settings() -> Settings:

[Benjamin-Nussbaum]

Suggested change

@lru_cache

[marcosfrenkel]

that was dumb of me 😅