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

Make red the default dataset visibility choice #226

Merged
merged 1 commit into from
Jun 14, 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

* New commands `okdata -e` and `okdata -v` for printing the current environment
and the current version of okdata-cli, respectively.
* The default choice for dataset visibility is now non-public (red). Also warn
when a dataset is about to be registered as public (green).

## 4.1.0 - 2024-04-22

Expand Down
21 changes: 11 additions & 10 deletions okdata/cli/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,6 @@ def print(self, str, payload=None):
else:
print(payload)

def confirm_to_continue(self, message):
"""Ask the user for confirmation before continuing.

Any answer other than "y" will exit the program.
"""
self.print(message)
if input("Continue? [y/N]: ") != "y":
self.print("Abort.")
sys.exit()

def login(self):
self.sdk.login()

Expand Down Expand Up @@ -148,6 +138,17 @@ def print_error_response(self, response_body):
print(response_body)


def confirm_to_continue(message):
"""Ask the user for confirmation before continuing.

Any answer other than "y" will exit the program.
"""
print(message)
if input("Continue? [y/N]: ") != "y":
print("Abort.")
sys.exit()


def _format_error_message(message, errors=None):
msg = f"An error occurred: {message}"
sep = "\n - "
Expand Down
4 changes: 2 additions & 2 deletions okdata/cli/commands/datasets/questions.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ def qs_create():
"name": "accessRights",
"message": "Tilgangsnivå",
"choices": [
Choice("Eksplisitt tilgangskontroll", "non-public"),
Choice("Internt til Origo", "restricted"),
Choice("Offentlig", "public"),
Choice("Begrenset offentlighet", "restricted"),
Choice("Unntatt offentlighet", "non-public"),
],
},
{
Expand Down
12 changes: 12 additions & 0 deletions okdata/cli/commands/datasets/wizards.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from okdata.sdk.data.dataset import Dataset
from okdata.sdk.pipelines.client import PipelineApiClient

from okdata.cli.command import confirm_to_continue
from okdata.cli.commands.datasets.questions import qs_create
from okdata.cli.commands.wizard import run_questionnaire

Expand Down Expand Up @@ -56,6 +57,17 @@ def start(self):
env = self.command.opt("env")
choices = run_questionnaire(*qs_create())

confirm_to_continue(
"Will create a new dataset '{}'.{}".format(
choices["title"],
(
"\n\nData uploaded to the dataset will be PUBLICLY AVAILABLE ON THE INTERNET.\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Blir kanskje for mye å fargelegge litt her?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Det skader vel ikke! Men kanskje det da kan gjøres i en egen PR hvor andre lignende advarsler også fargeleges. Kanskje fet skrift kunne holdt når jeg tenker meg om. 😛

if choices["accessRights"] == "public"
else ""
),
)
)

self.command.print("Creating dataset...")
dataset_client = Dataset(env=env)
dataset_config = self.dataset_config(choices)
Expand Down
12 changes: 5 additions & 7 deletions okdata/cli/commands/pubs/pubs.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from okdata.sdk.team.client import TeamClient

from okdata.cli import MAINTAINER
from okdata.cli.command import BASE_COMMAND_OPTIONS, BaseCommand
from okdata.cli.command import BASE_COMMAND_OPTIONS, BaseCommand, confirm_to_continue
from okdata.cli.commands.pubs.client import PubsClient
from okdata.cli.commands.pubs.questions import (
NoClientsError,
Expand Down Expand Up @@ -88,7 +88,7 @@ def create_client(self):
scopes = config["scopes"]
env = config["env"]

self.confirm_to_continue(
confirm_to_continue(
"Will create a new {} client{} in {}{}.".format(
client_types[client_type_id],
f" for {providers[provider_id]}" if provider_id else "",
Expand Down Expand Up @@ -138,7 +138,7 @@ def delete_client(self):
aws_account = choices["aws_account"]
aws_region = choices["aws_region"]

self.confirm_to_continue(
confirm_to_continue(
"Will delete client '{}' [{}]{}.".format(
client_name,
env,
Expand Down Expand Up @@ -241,7 +241,7 @@ def create_client_key(self):
aws_region = config["aws_region"]
enable_auto_rotate = config["enable_auto_rotate"]

self.confirm_to_continue(
confirm_to_continue(
"\n\n".join(
[
"Will create a new key for client '{}' in {} and {}.".format(
Expand Down Expand Up @@ -327,9 +327,7 @@ def delete_client_key(self):
client_name = choices["client_name"]
key_id = choices["key_id"]

self.confirm_to_continue(
f"Will delete key '{key_id}' from '{client_name}' [{env}]."
)
confirm_to_continue(f"Will delete key '{key_id}' from '{client_name}' [{env}].")

self.print(f"Deleting key '{key_id}' from '{client_name}' [{env}]...")
self.pubs_client.delete_key(env, client_id, key_id)
Expand Down
10 changes: 4 additions & 6 deletions okdata/cli/commands/teams/teams.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from okdata.sdk.team.client import TeamClient

from okdata.cli import MAINTAINER
from okdata.cli.command import BASE_COMMAND_OPTIONS, BaseCommand
from okdata.cli.command import BASE_COMMAND_OPTIONS, BaseCommand, confirm_to_continue
from okdata.cli.commands.teams.questions import NoTeamError
from okdata.cli.commands.teams.util import (
member_representation,
Expand Down Expand Up @@ -122,9 +122,7 @@ def add_member(self):
)
return

self.confirm_to_continue(
"Add {} to the team?".format(member_representation(user))
)
confirm_to_continue("Add {} to the team?".format(member_representation(user)))

self.client.update_team_members(
config["team_id"], team_members + [user["username"]]
Expand All @@ -150,13 +148,13 @@ def remove_member(self):
members_to_keep.append(member)

if len(members_to_keep) == 0:
self.confirm_to_continue(
confirm_to_continue(
"You are about to delete all members from the team, including "
"yourself. It will not be possible to edit this team any "
"further without being re-added by a systems administrator."
)
else:
self.confirm_to_continue(
confirm_to_continue(
"Remove the following member{} from the team?\n - {}".format(
"s" if len(members_to_remove) > 1 else "",
"\n - ".join(
Expand Down
14 changes: 8 additions & 6 deletions tests/origocli/commands/teams_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from unittest.mock import ANY, MagicMock
from unittest.mock import ANY, MagicMock, patch

import pytest
from requests import HTTPError
Expand Down Expand Up @@ -35,7 +35,6 @@ def make_cmd(mocker, *args):
set_argv("teams", *args)
cmd = teams.TeamsCommand()
mocker.patch.object(cmd, "client")
mocker.patch.object(cmd, "confirm_to_continue")
mocker.patch.object(teams, "list_members_wizard")
mocker.patch.object(teams, "add_member_wizard")
mocker.patch.object(teams, "remove_member_wizard")
Expand Down Expand Up @@ -88,7 +87,8 @@ def test_add_team_member(mocker, mock_print):
config = {"team_id": "team1", "username": "misty"}
teams.add_member_wizard.return_value = config

cmd.handler()
with patch("okdata.cli.commands.teams.teams.confirm_to_continue"):
cmd.handler()

teams.add_member_wizard.assert_called_once()
target_members = [u["username"] for u in mock_members]
Expand Down Expand Up @@ -149,7 +149,8 @@ def test_remove_team_member(mocker, mock_print):
config = {"team_id": team_id, "usernames": ["homersimpson"]}
teams.remove_member_wizard.return_value = config

cmd.handler()
with patch("okdata.cli.commands.teams.teams.confirm_to_continue"):
cmd.handler()

teams.remove_member_wizard.assert_called_once()
cmd.client.update_team_members.assert_called_once_with(
Expand All @@ -158,7 +159,8 @@ def test_remove_team_member(mocker, mock_print):
assert mock_print.mock_calls[0][1][0] == "Done!"


def test_remove_all_team_members(mocker, mock_print):
@patch("okdata.cli.commands.teams.teams.confirm_to_continue")
def test_remove_all_team_members(confirm_to_continue, mocker, mock_print):
cmd = make_cmd(mocker, "remove-member")
team_id = "team1"
config = {"team_id": team_id, "usernames": ["homersimpson", "misty", "janedoe"]}
Expand All @@ -167,6 +169,6 @@ def test_remove_all_team_members(mocker, mock_print):
cmd.handler()

teams.remove_member_wizard.assert_called_once()
cmd.confirm_to_continue.assert_called_once()
confirm_to_continue.assert_called_once()
cmd.client.update_team_members.assert_called_once_with(config["team_id"], [])
assert mock_print.mock_calls[0][1][0] == "Done!"
Loading