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

Add --strict option to CLI #132

Merged
merged 2 commits into from
Apr 26, 2024
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
4 changes: 2 additions & 2 deletions .github/workflows/ci_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ jobs:

- name: Upload coverage
if: github.repository_owner == 'SINTEF'
uses: codecov/codecov-action@v3
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
files: coverage.xml
Expand Down Expand Up @@ -185,7 +185,7 @@ jobs:

- name: Upload coverage
if: github.repository_owner == 'SINTEF'
uses: codecov/codecov-action@v3
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
fail_ci_if_error: true
Expand Down
15 changes: 15 additions & 0 deletions entities_service/cli/commands/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,20 @@ def upload(
show_default=False,
),
] = False,
strict: Annotated[
bool,
typer.Option(
"--strict",
help=(
"Strict validation of entities. This means the command will fail "
"during the validation process, if an external entity already exists "
"and the two entities are not equal. This option is only relevant if "
"'--no-external-calls' is not provided. If both '--no-external-calls'"
" and this options is provided, an error will be emitted."
),
show_default=True,
),
] = False,
) -> None:
"""Upload (local) entities to a remote location."""
# Ensure the user is logged in
Expand All @@ -159,6 +173,7 @@ def upload(
no_external_calls=False,
return_full_info=True,
verbose=False,
strict=strict,
)

# Sanity check - done only for typing to be caught by mypy and testing
Expand Down
89 changes: 67 additions & 22 deletions entities_service/cli/commands/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,20 @@ def validate(
show_default=True,
),
] = False,
strict: Annotated[
bool,
typer.Option(
"--strict",
help=(
"Strict validation of entities. This means validation will fail if "
"an external entity already exists and the two entities are not equal. "
"This option is only relevant if '--no-external-calls' is not provided."
" If both '--no-external-calls' and this options is provided, an error "
"will be emitted."
),
show_default=True,
),
] = False,
# Hidden options - used only when calling the function directly
return_full_info: Annotated[
bool,
Expand All @@ -171,9 +185,7 @@ def validate(
unique_filepaths = set(filepaths or [])
unique_directories = set(directories or [])

# Handle YAML/YML file format, ensuring both YAML and YML are in the set
if unique_file_formats & {EntityFileFormats.YAML, EntityFileFormats.YML}:
unique_file_formats |= {EntityFileFormats.YAML, EntityFileFormats.YML}
## Initial checks

if not (sources or filepaths or directories):
ERROR_CONSOLE.print(
Expand All @@ -193,6 +205,17 @@ def validate(
"Please, use a SOURCE instead."
)

if no_external_calls and strict:
ERROR_CONSOLE.print(
"[bold red]Error[/bold red]: The options '--no-external-calls' and "
"'--strict' can not be used together."
)
raise typer.Exit(1)

# Handle YAML/YML file format, ensuring both YAML and YML are in the set
if unique_file_formats & {EntityFileFormats.YAML, EntityFileFormats.YML}:
unique_file_formats |= {EntityFileFormats.YAML, EntityFileFormats.YML}

## Consolidate provided directories and filepaths

# stdin
Expand Down Expand Up @@ -394,14 +417,33 @@ def validate(
successes.append(ValidEntity(entity_model, True, True, None))
else:
# Record the differences between the external and local entities
successes.append(
ValidEntity(
entity_model,
True,
False,
pretty_compare_dicts(external_entity, dumped_entity),
pretty_diff = pretty_compare_dicts(external_entity, dumped_entity)
successes.append(ValidEntity(entity_model, True, False, pretty_diff))

if strict:
ERROR_CONSOLE.print(
f"[bold red]Error[/bold red]: Entity {get_uri(entity_model)} "
"already exists externally and differs in its contents."
)
)

if fail_fast:
if verbose:
ERROR_CONSOLE.print(
"\n[bold blue]Detailed differences:[/bold blue]"
)
ERROR_CONSOLE.print(
"", Rule(title=get_uri(entity_model)), f"\n{pretty_diff}\n"
)
elif not quiet and not return_full_info:
ERROR_CONSOLE.print(
"\n[bold blue]Use the option '--verbose' to see the "
"difference between the external and local entity."
"[/bold blue]\n"
)

raise typer.Exit(1)

failed_entities.append(get_uri(entity_model))

## Report the results

Expand Down Expand Up @@ -448,17 +490,16 @@ def validate(
if successes:
# List the validated entities in a table
table = Table(
expand=True,
box=box.HORIZONTALS,
show_edge=False,
highlight=True,
)

table.add_column("Namespace", no_wrap=True)
table.add_column("Namespace", no_wrap=False)
table.add_column("Name", no_wrap=True)
table.add_column("Version", no_wrap=True)
table.add_column("Exists externally", no_wrap=True)
table.add_column("Equal to external", no_wrap=True)
table.add_column("Exists externally", no_wrap=False)
table.add_column("Equal to external", no_wrap=False)

# Sort the entities in the following order:
# 1. Namespace
Expand Down Expand Up @@ -486,7 +527,11 @@ def validate(
],
{
True: "Yes",
False: "No",
False: (
"[bold red]No[/bold red] (error in strict-mode)"
if strict
else "No"
),
None: "Unknown" if no_external_calls else "-",
}[valid_entity.equal_to_remote],
)
Expand Down Expand Up @@ -525,11 +570,11 @@ def validate(
"sources.[/bold yellow]\n"
)

if not (failed_filepaths or failed_entities):
return (
successes
if return_full_info
else [valid_entity.entity for valid_entity in successes] # type: ignore[misc]
)
if failed_filepaths or failed_entities:
raise typer.Exit(1)

raise typer.Exit(1)
return (
successes
if return_full_info
else [valid_entity.entity for valid_entity in successes] # type: ignore[misc]
)
91 changes: 91 additions & 0 deletions tests/cli/commands/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -920,3 +920,94 @@ def test_using_stdin(
assert (
result.stdout.count("No") == number_of_valid_entities
), CLI_RESULT_FAIL_MESSAGE.format(stdout=result.stdout, stderr=result.stderr)


@pytest.mark.usefixtures("_mock_successful_oauth_response")
@pytest.mark.parametrize("fail_fast", [True, False], ids=["fail-fast", "no-fail-fast"])
def test_upload_strict(
cli: CliRunner,
static_dir: Path,
httpx_mock: HTTPXMock,
fail_fast: bool,
) -> None:
"""Test the `strict` flag."""
import json

from entities_service.cli.main import APP
from entities_service.service.config import CONFIG

valid_entity = static_dir / "valid_entities" / "Person.json"
raw_entity: dict[str, Any] = json.loads(valid_entity.read_bytes())

# Change the entity's content to make it different from the file
assert "dimensions" in raw_entity
assert isinstance(raw_entity["dimensions"], dict)
assert "n_skills" in raw_entity["dimensions"]

new_n_skills_description = "Skill number."
assert raw_entity["dimensions"]["n_skills"] != new_n_skills_description

changed_entity = raw_entity.copy()
changed_entity["dimensions"]["n_skills"] = new_n_skills_description

# Mock a good login check
httpx_mock.add_response(
url=f"{str(CONFIG.base_url).rstrip('/')}/_admin/create",
status_code=204,
match_json=[],
)

# Mock response for "Check if entity already exists"
assert changed_entity["uri"] is not None
httpx_mock.add_response(
url=changed_entity["uri"],
status_code=200, # ok
json=changed_entity,
)

result = cli.invoke(
APP,
f"upload --strict {'--fail-fast ' if fail_fast else ''}{valid_entity}",
)

assert result.exit_code == 1, CLI_RESULT_FAIL_MESSAGE.format(
stdout=result.stdout, stderr=result.stderr
)

assert (
"already exists externally and differs in its contents." in result.stderr
), CLI_RESULT_FAIL_MESSAGE.format(stdout=result.stdout, stderr=result.stderr)
assert (
"No entities were uploaded." not in result.stdout
), CLI_RESULT_FAIL_MESSAGE.format(stdout=result.stdout, stderr=result.stderr)

if fail_fast:
assert (
"Failed to validate one or more entities. See above for more details."
not in result.stderr
), CLI_RESULT_FAIL_MESSAGE.format(stdout=result.stdout, stderr=result.stderr)
assert "Valid Entities" not in result.stdout, CLI_RESULT_FAIL_MESSAGE.format(
stdout=result.stdout, stderr=result.stderr
)
else:
assert (
"Failed to validate one or more entities. See above for more details."
in result.stderr
), CLI_RESULT_FAIL_MESSAGE.format(stdout=result.stdout, stderr=result.stderr)
assert "Valid Entities" in result.stdout, CLI_RESULT_FAIL_MESSAGE.format(
stdout=result.stdout, stderr=result.stderr
)

assert (
result.stdout.replace(" ", "")
.replace("\n", "")
.count("No(errorinstrict-mode)")
== 1
), CLI_RESULT_FAIL_MESSAGE.format(stdout=result.stdout, stderr=result.stderr)

assert "Detailed diferences" not in (
result.stderr if fail_fast else result.stdout
), CLI_RESULT_FAIL_MESSAGE.format(stdout=result.stdout, stderr=result.stderr)
assert "Use the option '--verbose'" not in (
result.stderr if fail_fast else result.stdout
), CLI_RESULT_FAIL_MESSAGE.format(stdout=result.stdout, stderr=result.stderr)
Loading