Skip to content

Conversation

@brandon-wada
Copy link
Collaborator

No description provided.

Copy link
Contributor

@paulina-positronix paulina-positronix left a comment

Choose a reason for hiding this comment

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

It seems like the main change is inside experimental_api.py which LGTM. I'm not sure about the rest of the generated API code and especially the empty tests.


def testEdgeModelInfo(self):
"""Test EdgeModelInfo"""
# FIXME: construct object with mandatory attributes with example values
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to be replaced with an actual test?

pass

def test_get_model_urls(self):
"""Test case for get_model_urls"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to be replaced with an actual test?


def testModelUrl(self):
"""Test ModelUrl"""
# FIXME: construct object with mandatory attributes with example values
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to be replaced with an actual test?

def _download_mlbinary_url(self, detector: Union[str, Detector]) -> EdgeModelInfo:
"""
Gets a temporary presigned URL to download the model binaries for the given detector, along
with relevant metadata
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need secrets or anything to do this? Or is that taken care of here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The secrets are taken care of in the backend after you've authenticated with your api token. So long as you have the feature enabled in your Groundlight account, you should be able to download

@brandon-wada
Copy link
Collaborator Author

It seems like the main change is inside experimental_api.py which LGTM. I'm not sure about the rest of the generated API code and especially the empty tests.

True that the generated tests aren't particularly useful. Nothing inside the generated folder is written by hand, and in fact editing those files manually is very likely to get those changes overwritten, which is why we don't update the tests there.

@brandon-wada brandon-wada merged commit f8d89aa into main Feb 10, 2025
8 checks passed
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