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

feat: using SnippetManager in the Service Details to create code snippet #556

Merged
merged 9 commits into from Mar 19, 2024

Conversation

axel7083
Copy link
Contributor

@axel7083 axel7083 commented Mar 14, 2024

What does this PR do?

This PR is using the SnippetManager in the frontend to generate code snippet.

Screenshot / video of UI

snippet-generator.mp4

What issues does this PR fix or reference?

Fixes #555 #403

How to test this PR?

  • write unit tests

@axel7083 axel7083 requested a review from a team as a code owner March 14, 2024 18:04
@axel7083 axel7083 changed the title Feature/snippet in service details feat: using SnippetManager in the Service Details to create code snippet Mar 14, 2024
@axel7083 axel7083 marked this pull request as draft March 14, 2024 18:05
@axel7083 axel7083 marked this pull request as ready for review March 15, 2024 11:05
@axel7083 axel7083 force-pushed the feature/snippet-in-service-details branch 2 times, most recently from 336e4b3 to e654a05 Compare March 15, 2024 13:11
@axel7083 axel7083 requested a review from feloy March 15, 2024 13:54
Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

LGTM

{/each}
</select>
{#if selectedVariant !== undefined}
<select
Copy link
Contributor

Choose a reason for hiding this comment

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

The select should be disable if the language has only one variant

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we keep the select, disable it and put a "N/A" for "Not Applicable"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes

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 would rather apply the suggestion of @jeffmaury as displaying N/A or Not Applicable could be weird, when we do have a name for the variant. Showing a hint, of the library used generally.

Here is why it look likes when it is disabled when only one variant.

image image image

@feloy
Copy link
Contributor

feloy commented Mar 18, 2024

When I try with curl, the path seems incorrect, as I get a 404. The path is / in the snippet, it seems it should be /v1/chat/completions

@axel7083 axel7083 force-pushed the feature/snippet-in-service-details branch from bd63c0c to 35d70cb Compare March 19, 2024 08:31
@axel7083
Copy link
Contributor Author

When I try with curl, the path seems incorrect, as I get a 404. The path is / in the snippet, it seems it should be /v1/chat/completions

Oh nice catch @feloy, thanks for noticing. I fixed it

image

Here is what is resulting when executing the curl command

image

Copy link
Contributor

@feloy feloy left a comment

Choose a reason for hiding this comment

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

great!

Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com>
@axel7083 axel7083 force-pushed the feature/snippet-in-service-details branch from 35d70cb to 627a3fe Compare March 19, 2024 08:54
@axel7083 axel7083 enabled auto-merge (squash) March 19, 2024 08:55
@axel7083 axel7083 merged commit 6112bc2 into containers:main Mar 19, 2024
4 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.

Using SnippetManager in Service details page
4 participants