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 example programs, with integration test cases, and documentation about standalone use #13

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

amotl
Copy link
Member

@amotl amotl commented Sep 10, 2023

About

This patch brings in two example programs originally contributed by @andnig and added to crate/cratedb-examples#84, into the examples folder here instead.

In this way, by adding them to the test suite, we can assure they do not stop working. Through additional assistance by Dependabot, this yields reasonable efficiency to keep the repository up to date with upstream developments, and at the same time, makes sure the corresponding example programs get updated along the lines.

Details

  • Add corresponding test cases, which validate that all three software artefacts (CrateDB, MLflow Tracking Server, experiment program) play well together.
  • Verify that recording experiments also works well in standalone mode, without using an MLflow Tracking Server at all.
  • Improve README about how to run experiments in standalone mode.

@amotl amotl changed the title Add example experiment program, and corresponding tests Add example experiment program, and corresponding test case Sep 10, 2023
@amotl amotl changed the title Add example experiment program, and corresponding test case Add example experiment program with integration test case Sep 10, 2023
@amotl amotl changed the title Add example experiment program with integration test case Add example programs, with integration test cases, and documentation about standalone use Sep 10, 2023
Comment on lines +50 to +64
def table_exists(table_name: str, schema_name: str = "doc") -> bool:
"""
Check if database table exists.
"""
conn = connect_database()
cursor = conn.cursor()
sql = (
f"SELECT table_name FROM information_schema.tables " # noqa: S608
f"WHERE table_name = '{table_name}' AND table_schema = '{schema_name}'"
)
cursor.execute(sql)
rowcount = cursor.rowcount
cursor.close()
conn.close()
return rowcount > 0
Copy link
Member Author

Choose a reason for hiding this comment

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

@seut: Regarding your remark at crate/cratedb-examples#84 (comment), I am not completely convinced that I like this alternative for each and every occasion.

While I see it is a cleaner approach from the implementation perspective, it falls short on usability, because a generic table_exists function would need to obtain the schema parameter separately, like outlined here.

Sometimes, you just do not have it, because the schema name is opaquely embedded within the database connection string.

Copy link
Member

Choose a reason for hiding this comment

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

@amotl What about using table_schema = CURRENT_SCHEMA?

Copy link
Member Author

Choose a reason for hiding this comment

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

It worked well, see #26. Thanks.

@amotl amotl marked this pull request as ready for review September 10, 2023 14:36
Copy link
Member

@hammerhead hammerhead left a comment

Choose a reason for hiding this comment

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

Looks fine to me, I will give it a try shortly and might have further feedback (from a user perspective) after that.

Base automatically changed from iterate-1 to main September 12, 2023 12:06
- Refactor and improve test infrastructure
- Improve README about standalone usage
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.

3 participants