Add C API to instantiate service with custom options - #14
Matthew Treinish (mtreinish) wants to merge 10 commits into
Conversation
This commit adds a new function qkrt_service_new_from_config() which adds the ability to specify custom options when creating a new service. This functionality was lacking form the api which was forcing users to either manually create a json file in a fixed path or use the python qiskit-ibm-runtime library to create the config file. This new function lets a user set any custom options involved in a service instance necessary and rely on the defaults otherwise if something is not specified. Closes Qiskit#12
Co-authored-by: Ian Hincks <ian.hincks@gmail.com>
In earlier commits this PR had a single `base_url` field, but that was only used for IAM. The client has to connect to 3 API services with 3 different URLs: IAM, IQP, and global search. This commit removes the single url option and splits it out into 3 different fields, one for each url.
Co-authored-by: Jim Garrison <jim@garrison.cc>
| prefix: Some("Bearer".to_string()), | ||
| }); | ||
| if let Some(ref user_config) = account.user_config { | ||
| if let Some(ref base_path) = user_config.global_search_url { |
There was a problem hiding this comment.
The custom user agent doesn't reach this global-search request. A few lines up, config.user_agent is still hardcoded to "qiskit-ibm-runtime-rs/0.0.1" and user_config.user_agent is never consulted here — only global_search_url is applied from user_config. So a caller who sets config.user_agent gets it applied to the IAM (get_account) and IQP (Service::new) requests, but the Global Search request still goes out with the default. This looks like the same one-call-site-missed situation as the earlier "Fix setting user agent for ibm-quantum-platform requests too" commit, just for the third URL. Since the custom user agent is the headline example for this API, it'd be good to plumb it through here too.
This comment was generated by Claude Opus 4.8 under my guidance.
| * ``qkrt_default_service_config``. | ||
| */ | ||
| typedef struct { | ||
| /// The token to use to authenticate against IBM cloud with. By default it will be read from the |
There was a problem hiding this comment.
This doc ("By default it will be read from the specified config file") reads as though setting token lets you skip the config file, but I don't think that actually works today. get_account unconditionally calls get_account_config, which does File::open(path).unwrap() (and serde_json::from_reader(...).unwrap()) — so with an explicit token but no ~/.qiskit/qiskit-ibm.json present, the call panics/aborts across the FFI boundary rather than using the token and returning an error code.
For reference, the Python client explicitly supports token-only construction with no file on disk (QiskitRuntimeService(channel=..., token=...) logs "A saved account will not be used"), which is close to the motivation in the PR description — not forcing users to create a JSON file. Two options: wire token to bypass the file read when it's set, or adjust the docs to state that a config file is currently still required. (The .unwrap()-panic style predates this PR, so this is more a design gap than a regression — flagging it because the new token field is what surfaces it.)
This comment was generated by Claude Opus 4.8 under my guidance.
| /// default it will be "https://iam.cloud.ibm.com". | ||
| const char *iam_url; | ||
| /// The url to use for connecting to the IBM Quantum Platform. If this is a null pointer by | ||
| /// default it will be "https://quantum.cloud.ibm.com". |
There was a problem hiding this comment.
Minor doc nit: https://quantum.cloud.ibm.com is region-specific. The Python client derives this host per-instance from the account's region/CRN — e.g. an eu-de account resolves to https://eu-de.quantum.cloud.ibm.com/api/v1, and private endpoints get a private. prefix. So a non-us-east account left on the default here would point at the wrong endpoint. This region derivation is a pre-existing limitation of the Rust client (main doesn't do it either), so it's not something this PR needs to solve — but it might be worth softening the wording so the documented default isn't presented as universal. Same applies to the global_search_url default just below.
This comment was generated by Claude Opus 4.8 under my guidance.
| res = qkrt_job_status(&status, service, job); | ||
| if (res != 0) { | ||
| printf("status poll failed with code: %d\n", res); | ||
| goto cleanup; |
There was a problem hiding this comment.
Minor / tangential (and pre-existing — this sample is closely modeled on test_ghz_sampler_run.c, which has the same structure): this goto cleanup on a status-poll failure jumps straight to the label that only frees qc, so it skips freeing results, target, service, and job. The cleanup labels below are ordered for the happy-path fall-through, so an error here wants a label that unwinds the full chain rather than cleanup. Not a blocker for this PR — noting it since it's carried into the new copy. (The copy does fix two typos the original still has, for what it's worth.)
This comment was generated by Claude Opus 4.8 under my guidance.
| // Filter-out any instance that doesn't match the user's config. | ||
| instances.retain(|x| x.crn.to_str().unwrap() == instance); | ||
| } | ||
| *out = Box::into_raw(Box::new(Service::new(account, instances, user_agent))); |
There was a problem hiding this comment.
Very minor / tangential: user_agent gets plumbed in twice here — once inside account_config (stored on account.user_config and used by get_account for IAM) and again as this third argument to Service::new. It works, but the sourcing is a little inconsistent: Service::new reads iqp_url from account.user_config while taking user_agent as a separate parameter. Could simplify by having Service::new read user_agent from account.user_config too and dropping the extra arg.
This comment was generated by Claude Opus 4.8 under my guidance.
Jim Garrison (garrison)
left a comment
Jim Garrison (garrison)
left a comment
There was a problem hiding this comment.
I think that my comments are fairly minor and could be fixed in follow-up pull requests. I'm going to approve now so that we can get this functionality out to users sooner rather than later.
3 participants
This commit adds a new function qkrt_service_new_from_config() which adds the ability to specify custom options when creating a new service. This functionality was lacking form the api which was forcing users to either manually create a json file in a fixed path or use the python qiskit-ibm-runtime library to create the config file. This new function lets a user set any custom options involved in a service instance necessary and rely on the defaults otherwise if something is not specified.
Closes #12