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

Tox, Pipenv, Docker (-compose) and a sketch of a client cli #35

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jrabbit
Copy link

@jrabbit jrabbit commented Nov 6, 2018

Hey hows it going; I moved a bunch of stuff into a new workflow let me know how you feel about it!
The client isn't done yet but I'd like some feedback first. I need to add the dist/dev tools to the pipfile still (or you can as an exercise 😉)
I also had some issues with pytest sugar

@jrabbit
Copy link
Author

jrabbit commented Nov 13, 2018

Does the client stuff look good-ish? Should I keep going with that? @rsalmei

@rsalmei
Copy link
Owner

rsalmei commented Feb 13, 2019

Hello @jrabbit, how are you?
I'm so sorry for the delay, was away for a while...

Anyway, I'll take a more in depth look in your PR, but I do like the client stuff!
Some things we would need to tweek a little, like maintainer and docker-hub 😄
And finally, I'm not sold on pipenv yet, don't know if it would be nice to have in here yet.

Would you mind merging into another branch of mine, and I made the tweaks in there?

@nvictor
Copy link

nvictor commented Aug 29, 2019

Hey guys, I also tried to make this tool work in a container with no luck. I didn't notice this PR and will give it a try! My main issue is with gRPC, which I don't know well. I just can't make the client connect to the server inside the container. All I get is connection refused... I have been pulling my hair for an entire day now.

@rsalmei
Copy link
Owner

rsalmei commented Sep 4, 2019

Hey @nvictor !
Probably you only need to expose the container port to the host. Do a docker ps and verify the ports section.
I need to find some time to get back to this... I have my own implementation of docker image that is almost completed...

Copy link

@fmartins fmartins left a comment

Choose a reason for hiding this comment

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

@jrabbit: Congratulates for your initiative :)

I added some commentaries in the PR, could you please take a look at it?

LABEL maintainer "Jack Laxson <[email protected]>"

RUN mkdir -p /usr/src/clearly /usr/share/cache/ && \
chmod 777 /usr/share/cache/
Copy link

@fmartins fmartins Nov 2, 2019

Choose a reason for hiding this comment

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

🚫 777

  • We should run the clearly as a non-privileged user.

@fmartins
Copy link

fmartins commented Nov 2, 2019

@rsalmei: I loved the @jrabbit initiative, I found it before proposing something similar.
What do you think about it?

I would like to have a clearly official container to use in my infrastructure.

In addition to the @jrabbit example, I think that we should consider:

  • using the broker as an environment variable in the server-side;
  • using the server-side URL as an environment variable in the client-side.

@rsalmei rsalmei force-pushed the master branch 5 times, most recently from 6e5f9d0 to d7f847f Compare April 1, 2020 13:25
@rsalmei rsalmei force-pushed the master branch 8 times, most recently from c8770b7 to 8984a89 Compare April 14, 2020 08:52
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.

4 participants