-
-
Notifications
You must be signed in to change notification settings - Fork 184
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
fix: regenerate schema #1993
fix: regenerate schema #1993
Conversation
@ytausch - let me know what's up here :) I think the changes got lost after the |
@@ -11,19 +11,19 @@ azure: | |||
pool: | |||
vmImage: ubuntu-latest | |||
swapfile_size: 0GiB | |||
timeoutInMinutes: 360 | |||
timeout_in_minutes: 360 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit confused how this will affect us. This change appeared because the corresponding Pydantic model field was renamed from timeoutInMinutes
to timeout_in_minutes
in the ruff PR, so that is right, @wolfv.
To not change our schema though, I set this alias here:
conda-smithy/conda_smithy/schema.py
Lines 111 to 115 in 0bf152c
timeout_in_minutes: Optional[int] = Field( | |
default=360, | |
description="Timeout in minutes for the job", | |
alias="timeoutInMinutes", | |
) |
However, because we save the default values with model_dump
, which by default does not respect aliases this defaults yaml file now has a different field name.
This actually breaks the logic setting field defaults, I suppose:
conda-smithy/conda_smithy/configure_feedstock.py
Lines 2170 to 2205 in 0bf152c
def _read_forge_config(forge_dir, forge_yml=None): | |
# Load default values from the conda-forge.yml file | |
with open(CONDA_FORGE_YAML_DEFAULTS_FILE) as fh: | |
default_config = yaml.safe_load(fh.read()) | |
if forge_yml is None: | |
forge_yml = os.path.join(forge_dir, "conda-forge.yml") | |
if not os.path.exists(forge_yml): | |
raise RuntimeError( | |
f"Could not find config file {forge_yml}." | |
" Either you are not rerendering inside the feedstock root (likely)" | |
" or there's no `conda-forge.yml` in the feedstock root (unlikely)." | |
" Add an empty `conda-forge.yml` file in" | |
" feedstock root if it's the latter." | |
) | |
with open(forge_yml) as fh: | |
documents = list(yaml.safe_load_all(fh)) | |
file_config = (documents or [None])[0] or {} | |
# Validate loaded configuration against a JSON schema. | |
validate_lints, validate_hints = validate_json_schema(file_config) | |
for err in chain(validate_lints, validate_hints): | |
logger.warning( | |
"%s: %s = %s -> %s", | |
os.path.relpath(forge_yml, forge_dir), | |
err.json_path, | |
err.instance, | |
err.message, | |
) | |
logger.debug("Relevant schema:\n%s", json.dumps(err.schema, indent=2)) | |
# The config is just the union of the defaults, and the overridden | |
# values. | |
config = _update_dict_within_dict(file_config.items(), default_config) |
I think merging this might have broken conda-smithys default values. Another reason why using Pydantic as a pure JSON schema generator is not a good idea, xref #1920 |
See the PR which is linked above. |
I ran
python conda_smithy/schema.py
.