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

chore: Remove endpoint date_updated #3711

Merged
merged 5 commits into from Mar 31, 2023
Merged

chore: Remove endpoint date_updated #3711

merged 5 commits into from Mar 31, 2023

Conversation

liuverta
Copy link
Contributor

@liuverta liuverta commented Mar 30, 2023

Impact and Context

It currently provides no user-facing value [Slack].

Risks and Area of Effect

Low; I'm skeptical anyone was even paying it any mind through the client.

Testing

  • Unit test
  • Deployed to dev env
  • Other (explain)

Oops not a unit test, but

% pytest test_cli/test_endpoint.py::TestGet::test_get test_endpoint/test_endpoint.py::TestEndpoint::test_repr
==================================================== test session starts ====================================================
platform darwin -- Python 3.10.6, pytest-7.2.1, pluggy-1.0.0
rootdir: /Users/miliu/Documents/modeldb/client/verta/tests, configfile: pytest.ini
plugins: forked-1.4.0, xdist-3.1.0, typeguard-2.13.3, hypothesis-6.67.1
collected 2 items                                                                                                           

test_cli/test_endpoint.py .                                                                                           [ 50%]
test_endpoint/test_endpoint.py .                                                                                      [100%]

========================================= 2 passed, 4 warnings in 373.39s (0:06:13) =========================================

Reverting

  • Contains Migration - Do Not Revert

Comment on lines -129 to -130
def _date_updated(self, data):
return data["date_updated"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I searched the codebase for \._date_updated and it was only being used in this module, below.

@@ -202,7 +201,7 @@ def _get_by_id(cls, conn, conf, workspace, id):

def _get_info_list_by_id(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I searched the codebase for \._get_info_list_by_id and it's only being used in _cli/deployment/list.py.

table_data = [["PATH", "ID", "DATE UPDATED"]] + list(
table_data = [["PATH", "ID", "DATE CREATED"]] + list(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now:

% verta deployment list endpoint
got VERTA_HOST from environment
got VERTA_EMAIL from environment
got VERTA_DEV_KEY from environment
connection successfully established

PATH             ID   DATE CREATED               
/simple-deploy   1    2023-03-29T20:52:42.000Z   

Comment on lines -98 to +109
assert "path: {}".format(endpoint.path) in result.output
assert "id: {}".format(endpoint.id) in result.output
assert "curl: <endpoint not deployed>" in result.output

assert "status" in result.output
assert "date created" in result.output
assert "date updated" in result.output
assert "stage's date created" in result.output
assert "stage's date updated" in result.output
assert "components" in result.output

updated_status = endpoint.update(experiment_run, DirectUpdateStrategy(), True)

def in_output(s: str) -> bool:
return any(line.startswith(s) for line in result.output.splitlines())

assert in_output(f"path: {endpoint.path}")
assert in_output(f"id: {endpoint.id}")
assert in_output("curl: <endpoint not deployed>")
assert in_output("status")
assert in_output("date created")
assert in_output("stage's date created")
assert in_output("stage's date updated")
assert in_output("components")
Copy link
Contributor Author

@liuverta liuverta Mar 30, 2023

Choose a reason for hiding this comment

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

In this test (and the other test), I'm just

  • defining a closure to check the output using str.startswith() instead of in
  • using f-strings
  • removing "date updated"

@liuverta liuverta marked this pull request as ready for review March 30, 2023 18:29
@liuverta liuverta merged commit 08681e3 into main Mar 31, 2023
7 checks passed
@liuverta liuverta deleted the liu/endpoint-updated branch March 31, 2023 18:42
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