Skip to content
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: Invert default schema URLs #971

Merged
merged 1 commit into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/workflows/ci-fmudataio.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ jobs:
"fmu-sumo-uploader @ git+https://github.com/equinor/fmu-sumo-uploader.git"

- name: Full test
run: pytest -v --cov --cov-report term-missing
run: |
if [[ "${{ github.event.pull_request.base.ref }}" == "staging" ]]; then
pytest -n auto -v --cov --cov-report term-missing --prod
Comment on lines +46 to +47
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both pytest and update-schemas take a --prod flag now. Both ensure the DEV_SCHEMAS environment variable is unset. This causes the default case to happen: schemas and metadata are produced with the production url.

else
pytest -n auto -v --cov --cov-report term-missing
fi
docker_build:
name: Build Docker image
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/schemas-up-to-date.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ jobs:
pip install pip -U
pip install -e .
- name: Set SCHEMA_RELEASE for staging
if: ${{ github.event.pull_request.base.ref == 'staging' }}
run: echo "SCHEMA_RELEASE=1" >> $GITHUB_ENV

- name: Check schema
run: |
./tools/update-schema --diff
if [[ "${{ github.event.pull_request.base.ref }}" == "staging" ]]; then
./tools/update-schema --diff --prod
else
./tools/update-schema --diff
fi
- name: Ensure schema validates with AJV
run: |
Expand Down
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,11 @@ dev = [
"pyarrow-stubs",
"pydocstyle",
"PyQt5-sip<12.16.0; python_version == '3.8'",
"pytest",
"pytest-cov",
"pytest-mock",
"pytest-runner",
"pytest",
"pytest-xdist",
Comment on lines -63 to +64
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort + not sure why this dependency was never added to parallelize tests

"rstcheck",
"ruff",
"types-PyYAML",
Expand Down
6 changes: 3 additions & 3 deletions src/fmu/dataio/_models/_schema_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ def url(cls) -> str:
DEV_URL = f"{FmuSchemas.DEV_URL}/{cls.PATH}"
PROD_URL = f"{FmuSchemas.PROD_URL}/{cls.PATH}"

if os.environ.get("SCHEMA_RELEASE", False):
return PROD_URL
return DEV_URL
if os.environ.get("DEV_SCHEMA", False):
return DEV_URL
return PROD_URL
Comment on lines +141 to +143
Copy link
Collaborator Author

@mferrera mferrera Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't be able to produce metadata with the $schema = radix dev in Komodo bleeding without exploring some other option. I guess this is OK but not ideal. Could check for KOMODO_BLEEDING explicitly, but I don't really like that


@classmethod
def default_generator(cls) -> type[GenerateJsonSchema]:
Expand Down
22 changes: 22 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import logging
import os
import shutil
import sys
from copy import deepcopy
from pathlib import Path

Expand All @@ -14,6 +15,12 @@
import xtgeo
import yaml

# This must be set before anything in dataio is imported if not using pytest-xdist.
if "--prod" in sys.argv:
os.environ["DEV_SCHEMA"] = ""
else:
os.environ["DEV_SCHEMA"] = "1"

import fmu.dataio as dio
from fmu.config import utilities as ut
from fmu.dataio._models.fmu_results import FmuResults, fields, global_configuration
Expand Down Expand Up @@ -45,6 +52,21 @@
ERT_RUNPATH = f"_ERT_{FmuEnv.RUNPATH.name}"


def pytest_addoption(parser):
parser.addoption(
"--prod", action="store_true", help="Use schemas/metadata with production URLs."
)
Comment on lines +55 to +58
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes pytest aware of our custom pytest --prod command, configured below. But also set at the top of this file because pytest, and some tests, do funky things with environment variables. Since the models in dataio look for this environment variable it is safest to ensure it's set before they are imported, but are still set by the pytest config anyway



def pytest_configure(config) -> None:
"""If '--prod' is given to pytest, use schemas/metadata with prod urls (i.e. not dev
urls)."""
if config.getoption("--prod"):
os.environ["DEV_SCHEMA"] = ""
else:
os.environ["DEV_SCHEMA"] = "1"


def _current_function_name():
"""Helper to retrieve current function name, e.g. for logging"""
return inspect.currentframe().f_back.f_code.co_name
Expand Down
21 changes: 16 additions & 5 deletions tests/test_schema/test_schemas_up_to_date.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ def test_schemas_uptodate(schema: SchemaBase) -> None:
the local schema with the output of `dump()`.
To get more feedback or generate new schemas run:
`./tools/update_schema`
./tools/update_schema --diff
If you are generating a production release try running:
./tools/update_schema --diff --prod
"""
with open(schema.PATH) as f:
assert json.load(f) == schema.dump()
Expand All @@ -45,11 +50,17 @@ def test_schemas_uptodate(schema: SchemaBase) -> None:
def test_schema_url_changes_with_env_var(
schema: SchemaBase, monkeypatch: MonkeyPatch
) -> None:
assert schema.url().startswith(FmuSchemas.DEV_URL)
assert schema.dump()["$id"].startswith(FmuSchemas.DEV_URL)
monkeypatch.setenv("SCHEMA_RELEASE", "1")
monkeypatch.setenv("DEV_SCHEMA", "")
json = schema.dump()
assert schema.url().startswith(FmuSchemas.PROD_URL)
assert schema.dump()["$id"].startswith(FmuSchemas.PROD_URL)
assert json["$id"].startswith(FmuSchemas.PROD_URL)
assert json["$schema"] == "https://json-schema.org/draft/2020-12/schema"

monkeypatch.setenv("DEV_SCHEMA", "1")
json = schema.dump()
assert schema.url().startswith(FmuSchemas.DEV_URL)
assert json["$id"].startswith(FmuSchemas.DEV_URL)
assert json["$schema"] == "https://json-schema.org/draft/2020-12/schema"


@pytest.mark.parametrize("schema", schemas)
Expand Down
38 changes: 27 additions & 11 deletions tools/update-schema
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ import subprocess
import sys
from copy import deepcopy
from pathlib import Path
from typing import Any, Dict, List, TypeVar
from typing import TYPE_CHECKING, Any, Dict, List, TypeVar

from fmu.dataio._models import schemas
from fmu.dataio._models._schema_base import SchemaBase
if TYPE_CHECKING:
from fmu.dataio._models._schema_base import SchemaBase

GREEN = "\033[32m"
RED = "\033[31m"
Expand All @@ -47,6 +47,12 @@ def _get_parser() -> argparse.ArgumentParser:
action="store_true",
help="Show a diff between the current schema and the new one in output.",
)
parser.add_argument(
"-p",
"--prod",
action="store_true",
help="Produce schemas with production URLs",
)
parser.add_argument(
"-t",
"--test",
Expand Down Expand Up @@ -183,13 +189,21 @@ def write_schema(
_show_py_diff(existing_schema, new_schema)
return False

if is_release:
if (
is_release
and not force_overwrite
and existing_schema["$id"] != new_schema["$id"]
):
print(
INFO,
f"{BOLD}{schema.FILENAME}{NC} version {BOLD}{schema.VERSION}{NC}: "
f"modifying '$id' url to 'prod':\n {schema.url()}",
FAIL,
f"🚨 {BOLD}{schema.FILENAME}{NC} version "
f"{BOLD}{schema.VERSION}{NC}: mismatch in '$id'. you probably need "
"to run `./tools/update-schema --prod --force`",
)
elif new_schema == existing_schema:
if show_diff:
_show_py_diff(existing_schema, new_schema)
return False
if new_schema == existing_schema:
print(
PASS,
f"{BOLD}{schema.FILENAME}{NC} version "
Expand Down Expand Up @@ -219,11 +233,13 @@ def main() -> None:
if args.force:
print(INFO, "forcing overwrite of all schemas")

is_release = bool(os.environ.get("SCHEMA_RELEASE", False))
os.environ["DEV_SCHEMA"] = "" if args.prod else "1"
# Ensures URLs will differ based on above
import fmu.dataio._models as models
Comment on lines +237 to +238
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lazy import to ensure the models receive the DEV_SCHEMA env var we want them to


failed_a_write = False
for schema in schemas:
did_write = write_schema(schema, args.force, is_release, args.test, args.diff)
for schema in models.schemas:
did_write = write_schema(schema, args.force, args.prod, args.test, args.diff)
if not did_write:
failed_a_write = True

Expand Down
Loading