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

[OPIK-266] Replace requests library in the demo generator script #373

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

ferc
Copy link
Contributor

@ferc ferc commented Oct 11, 2024

Details

  • Replace requests library with urllib to avoid SSL issues in the lambda function

Issues

Resolves #

Testing

Documentation

@ferc ferc requested a review from a team as a code owner October 11, 2024 16:46
@ferc ferc changed the title [OPIK-266] Replace requests library in the demo generator script (issues in lambda function) [OPIK-266] Replace requests library in the demo generator script Oct 11, 2024
@jverre
Copy link
Collaborator

jverre commented Oct 13, 2024

@ferc Looks good to me, FYI we will be moving this script to use the Opik SDK to make it easier to maintain

Copy link
Collaborator

@andrescrz andrescrz left a comment

Choose a reason for hiding this comment

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

I assume this is working fine , as the PR description doesn't mention how it was tested. Otherwise, LGTM.

@ferc
Copy link
Contributor Author

ferc commented Oct 14, 2024

Yes, it was already tested, I'd like to keep the same logic here as the script we use for generating the demo projects in cloud, therefore for now it's still using the REST API but we can create a separate ticket for @alexkuzmik to transform both places to use Opik instead (low priority IMHO)

@ferc ferc merged commit 59afadd into main Oct 14, 2024
21 checks passed
@ferc ferc deleted the fernando/OPIK-266-replace-requests-library branch October 14, 2024 09:31
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