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

Recommendation Service Update - Python #92

Merged
merged 5 commits into from
Jun 7, 2022
Merged

Recommendation Service Update - Python #92

merged 5 commits into from
Jun 7, 2022

Conversation

mic-max
Copy link
Contributor

@mic-max mic-max commented Jun 2, 2022

Similar ideas of replacing generated code with generating such code in the Dockerfile and making environment variables a requirement rather than falling back onto a default value are intended to be done in other projects.

Changes

  • Replace pb2 generated files with them being compiled in the Dockerfile
  • Make port and depended on service environment variables required, otherwise program will throw an exception
  • Update client program to work
  • Delete genproto.sh

PS: This is the only python service that we intend to keep so that's why we shouldn't try to make this change to the emailservice

@mic-max mic-max requested a review from a team June 2, 2022 20:57
@mic-max mic-max changed the title Recommendation Service Update Recommendation Service Update - Python Jun 2, 2022
compose.yml Outdated Show resolved Hide resolved
src/recommendationservice/client.py Outdated Show resolved Hide resolved
Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

LGTM!

@cartersocha cartersocha merged commit 02cd941 into open-telemetry:main Jun 7, 2022
@mic-max mic-max deleted the recc-proto branch June 7, 2022 23:12
GaryPWhite pushed a commit to wayfair-contribs/opentelemetry-demo that referenced this pull request Jun 30, 2022
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
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