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

[ENHANCEMENT] CLI init command implemented for v3 api #2626

Merged
merged 15 commits into from Mar 31, 2021

Conversation

anthonyburdi
Copy link
Member

Changes proposed in this pull request:

  • Implement great_expectations init command for v3 (Batch Request) API
  • Current functionality does not mirror the v2 init CLI but instead creates the project configuration folder structure and example files, eliminating the interactivity.
  • Remove the --view option

# Conflicts:
#	docs/changelog.rst
@anthonyburdi anthonyburdi enabled auto-merge (squash) March 30, 2021 20:27
@anthonyburdi anthonyburdi enabled auto-merge (squash) March 30, 2021 20:48
Copy link
Contributor

@Aylr Aylr left a comment

Choose a reason for hiding this comment

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

Excellent work. I have a few nitpicks that will increase test maintainability by removing code from them.

Comment on lines -44 to -49
@click.option(
# Note this --no-view option is mostly here for tests
"--view/--no-view",
help="By default open in browser unless you specify the --no-view flag.",
default=True,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

YAY

Comment on lines -115 to -202
# Skip the rest of setup if --assume-yes flag is passed
if ctx.obj.assume_yes:
cli_message(SECTION_SEPARATOR)
cli_message(SETUP_SUCCESS)
sys.exit(0)

try:
# if expectations exist, offer to build docs
context = DataContext(ge_dir)
if context.list_expectation_suites():
if click.confirm(BUILD_DOCS_PROMPT, default=True):
build_docs(context, view=view)

else:
datasources = context.list_datasources()
if len(datasources) == 0:
cli_message(SECTION_SEPARATOR)
if not click.confirm(
"Would you like to configure a Datasource?", default=True
):
cli_message("Okay, bye!")
sys.exit(1)
datasource_name, data_source_type = add_datasource_impl(
context, choose_one_data_asset=False
)
if not datasource_name: # no datasource was created
sys.exit(1)

datasources = context.list_datasources()
if len(datasources) == 1:
datasource_name = datasources[0]["name"]

cli_message(SECTION_SEPARATOR)
if not click.confirm(
"Would you like to profile new Expectations for a single data asset within your new Datasource?",
default=True,
):
cli_message(
"Okay, exiting now. To learn more about Profilers, run great_expectations profile --help or visit docs.greatexpectations.io!"
)
sys.exit(1)

(
success,
suite_name,
profiling_results,
) = toolkit.create_expectation_suite(
context,
datasource_name=datasource_name,
additional_batch_kwargs={"limit": 1000},
flag_build_docs=False,
open_docs=False,
)

cli_message(SECTION_SEPARATOR)
if not click.confirm(
"Would you like to build Data Docs?", default=True
):
cli_message(
"Okay, exiting now. To learn more about Data Docs, run great_expectations docs --help or visit docs.greatexpectations.io!"
)
sys.exit(1)

build_docs(context, view=False)

if not click.confirm(
"\nWould you like to view your new Expectations in Data Docs? This will open a new browser window.",
default=True,
):
cli_message(
"Okay, exiting now. You can view the site that has been created in a browser, or visit docs.greatexpectations.io for more information!"
)
sys.exit(1)
toolkit.attempt_to_open_validation_results_in_data_docs(
context, profiling_results
)

cli_message(SECTION_SEPARATOR)
cli_message(SETUP_SUCCESS)
sys.exit(0)
except (
DataContextError,
ge_exceptions.ProfilerError,
OSError,
SQLAlchemyError,
) as e:
cli_message("<red>{}</red>".format(e))
sys.exit(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

👋 Bye!

),
]

# Assertions to make sure we didn't regress to v2 API
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow thorough!

[
("--v3-api init", "Y\n"),
("--v3-api --assume-yes init", ""),
],
)
@mock.patch("webbrowser.open", return_value=True, side_effect=None)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: Since init is so much simpler now we can remove the mock webbrowser and all it's assertions for maintainability in this an other init tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was leaving this in as a check on regression - but I think it's pretty safe to remove them and will probably speed up the tests so 👍

Comment on lines 122 to 329
not yet have an uncommitted directory.
"""
root_dir: str = str(tmp_path_factory.mktemp("hiya"))
root_dir_path = tmp_path / "hiya"
root_dir_path.mkdir()
root_dir = str(root_dir_path)
data_folder_path = os.path.join(root_dir, "data")
os.makedirs(os.path.join(root_dir, "data"))
data_folder_path: str = os.path.join(root_dir, "data")
data_path: str = os.path.join(root_dir, "data", "Titanic.csv")
data_path: str = os.path.join(data_folder_path, "Titanic.csv")
fixture_path: str = file_relative_path(
__file__, os.path.join("..", "test_sets", "Titanic.csv")
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: we don't need data fixtures for all init tests. Let's remove them for simplicity.

Copy link
Member Author

Choose a reason for hiding this comment

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

YES good call.

@anthonyburdi anthonyburdi requested a review from Aylr March 30, 2021 21:42
@anthonyburdi anthonyburdi merged commit 2aa2490 into develop Mar 31, 2021
@anthonyburdi anthonyburdi deleted the anthony/cli-update/init branch March 31, 2021 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants