Skip to content

Conversation

@guylei-code
Copy link
Contributor

OpenAI Module without notebook

Copy link
Collaborator

@danielperezz danielperezz left a comment

Choose a reason for hiding this comment

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

Looks good, a few comments :)

@@ -0,0 +1,17 @@
apiVersion: v1
categories:
- general
Copy link
Collaborator

Choose a reason for hiding this comment

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

general is only the kind, not a category. Try looking for appropriate categories from the hub filter menu.

@@ -0,0 +1,4 @@
fastapi>=0.110,<1.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

The requirements.txt file is used to configure dependencies for unit test. Since there is no unit test, this file is not needed.
The module’s dependencies are specified in the item.yaml, but based on the code, it seems the user doesn’t actually need to have them installed, is that correct? If so, there’s no need to specify them at all.

categories:
- general
description: OpenAI application runtime based on fastapi
example: openai_example.ipynb
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
example: openai_example.ipynb
example: openai.ipynb

mlrunVersion: 1.10.0
name: openai
spec:
filename: openai_app.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
filename: openai_app.py
filename: openai.py

@@ -0,0 +1,39 @@

Copy link
Collaborator

Choose a reason for hiding this comment

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

Stay consistent with the name of the module, rename this file to openai.py

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.

2 participants