From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5B29445462; Fri, 14 Jun 2024 20:32:56 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 356B640B9A; Fri, 14 Jun 2024 20:32:56 +0200 (CEST) Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) by mails.dpdk.org (Postfix) with ESMTP id 41ECA4060B for ; Fri, 14 Jun 2024 20:32:54 +0200 (CEST) Received: by mail-pj1-f43.google.com with SMTP id 98e67ed59e1d1-2c2ecbc109fso2121164a91.1 for ; Fri, 14 Jun 2024 11:32:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1718389973; x=1718994773; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=thlMRV0N2oSJjxttErG40XXjf8oJ9ID8q4nDRGZ2yTk=; b=H6pYDhSe61JpZwqFe/+c5K+W6M9JYUWe+vD5sWDalqkQBSiV8gMxUqDMBAfwEvU0XR KAmQRNP4Sa1o2q7ZFvIckbBuItvjz05IbFp5InuirQnZIkaWMjCbA0FlMv8oraV0QgNv 6PCxE+8SAbRZS6IvsCuhqC/xRglEWYoiPF8iM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718389973; x=1718994773; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=thlMRV0N2oSJjxttErG40XXjf8oJ9ID8q4nDRGZ2yTk=; b=j8Vh24r0p+qaoB8MOlnitT2H6XfJ6WHw6/w6O54bXpCoHcMWgDVZmF75o05v6M/Wdx r2aJdDNvulfgpJyxE5XfVooWEyME8ZUELGev4PEtVYhHkUZIlwnjzW4b9JAqKTUw3+7M dEzWehZpaLoSprQMKvtsm+dfPuYVHwFdH2sVtPyPcKd5E+ZLpfME1Nk4XG1cevq0vYia FeeqOJoY93VLOiY4vWE+OtPj4optEN3pQZpZHzpjZRw/4f6e3WSdSxwl13yPzO7jz9Xc kDhDQ77GgYfB6WOgMF7bBO8sRg8jLYGEj8HvPu3VgEzVJlNbF1aUwl2dJQgrGRWnh+lb W6XA== X-Forwarded-Encrypted: i=1; AJvYcCUwLe7zFWfZdzHkRy+D0HD6/ebB7M4qp0HKt5r0zORf9rGh7Zzl6r0ERonmdquEft66e+SZMcZ6O1Au0RI= X-Gm-Message-State: AOJu0YwqDBqbb7N5Er3nvDNobkb85l+pqXOKtaZd+qEPWYbxkDmfMiYM A7iI83xZh94Kmd6NeV0kO3/cGXo1wsyAW0XPuyoPgZR82u4wIUZzpJQkWALMy1PVEkZkeOtDJtL kOxE9Tv8NdQCQRpIt/U4a9E1511LWNUOa3x5UGg== X-Google-Smtp-Source: AGHT+IHUDF40nSPnfAAUvwd4xXG+RiYs76nO9TztMNlL4clDIob38//Gpa4ExiWSPbvNK09B2ycmHSbVmZ50gQpmuUg= X-Received: by 2002:a17:90b:2341:b0:2c3:cc8:3ee1 with SMTP id 98e67ed59e1d1-2c4dbd35594mr3950776a91.35.1718389973405; Fri, 14 Jun 2024 11:32:53 -0700 (PDT) MIME-Version: 1.0 References: <20240610193410.17968-2-npratte@iol.unh.edu> In-Reply-To: From: Jeremy Spewock Date: Fri, 14 Jun 2024 14:32:42 -0400 Message-ID: Subject: Re: [PATCH v1] dts: Testbed And Node Configuration Split To: Nicholas Pratte Cc: luca.vizzarro@arm.com, juraj.linkes@pantheon.tech, probb@iol.unh.edu, bruce.richardson@intel.com, yoan.picchi@foss.arm.com, paul.szczepanek@arm.com, Honnappa.Nagarahalli@arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Mon, Jun 10, 2024 at 3:37=E2=80=AFPM Nicholas Pratte wrote: > > > > > -def load_config(config_file_path: Path) -> Configuration: > > +def load_config(node_config_file_path: Path, exec_config_file_path: Pa= th) -> Configuration: > > """Load DTS test run configuration from a file. > > > > - Load the YAML test run configuration file > > + Load both the YAML testbed and execution configuration files > > and :download:`the configuration file schema `, > > - validate the test run configuration file, and create a test run co= nfiguration object. > > + validate both configuration files to create a test run configurati= on object. > > > > The YAML test run configuration file is specified in the :option:`= --config-file` command line > > argument or the :envvar:`DTS_CFG_FILE` environment variable. > > > > Args: > > - config_file_path: The path to the YAML test run configuration = file. > > + node_config_file_path: The path to the testbed configuration Y= AML file. > > + exec_config_file_path: The path to the execution configuration= YAML file. > > > > Returns: > > The parsed test run configuration. > > """ > > - with open(config_file_path, "r") as f: > > - config_data =3D yaml.safe_load(f) > > + with open(node_config_file_path, "r") as f: > > + node_config_data =3D yaml.safe_load(f) > > + with open(exec_config_file_path, "r") as f: > > + execution_config_data =3D yaml.safe_load(f) > > > > schema_path =3D os.path.join(Path(__file__).parent.resolve(), "con= f_yaml_schema.json") > > > > with open(schema_path, "r") as f: > > schema =3D json.load(f) > > - config =3D warlock.model_factory(schema, name=3D"_Config")(config_= data) > > - config_obj: Configuration =3D Configuration.from_dict(dict(config)= ) # type: ignore[arg-type] > > + config =3D { > > + **dict(warlock.model_factory(schema, name=3D"_Config")(node_co= nfig_data)), > > + **dict(warlock.model_factory(schema, name=3D"_Config")(executi= on_config_data)), > > + } > > + if "nodes" not in config or "executions" not in config: > > + raise ConfigurationError( > > + f"{'node' if 'nodes' not in config else 'execution'} data = not configured." > > + ) > > + config_obj: Configuration =3D Configuration.from_dict(config) # t= ype: ignore[arg-type] > > return config_obj > > There is an alternative approach to this that I currently have > implemented on a separate local branch. Essentially, both configs are > merged together before being validated by the schema, effectively > removing the need to change the schema at all as well as the above > assertion that both nodes and executions are in the two config files. > However, using this alternative method might mean that there is no added > control that prevents users from making funky errors when creating > both config files. I could also just be overthinking or missing > something. That's an interesting idea and on one hand it makes the schema a little less confusing because it wouldn't need to say "oneOf" when both of the attributes are actually required for DTS to run, but on the other it removes the ability for developers to know if they made a mistake without actually running DTS. Making it a runtime error wouldn't be the worst thing since it would at least be very early in the run that the error is shown, but it loses some of its purpose I feel if your IDE doesn't also show you some of the errors before actually using the config. I could go either way on it personally. > >