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

changes default logging behavior to not tail logs #807

Closed
wants to merge 3 commits into from

Conversation

btaitelb
Copy link

@btaitelb btaitelb commented Jan 6, 2015

Signed-off-by: Ben Taitelbaum ben@coshx.com

if tail:
pass
else:
break
Copy link

Choose a reason for hiding this comment

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

I think this could be simplified to just

if not tail:
    return

@btaitelb
Copy link
Author

btaitelb commented Jan 6, 2015

@dnephin thanks for the review. I've made the requested changes.

@dnephin
Copy link

dnephin commented Jan 6, 2015

Cool, lgtm for a major version release

Signed-off-by: Ben Taitelbaum <ben@coshx.com>
  - adds a `--tail` option to `fig logs`
  - see docker#802 for discussion of `--tail` vs `--no-tail`

Signed-off-by: Ben Taitelbaum <ben@coshx.com>
Signed-off-by: Ben Taitelbaum <ben@coshx.com>
@dnephin
Copy link

dnephin commented Oct 6, 2015

Thanks for the contribution!

Sorry it's taken so long to get a response.

I don't think this implementation is correct. It assumes that when the queue.get() times out that we've hit the end of the stream, but that isn't necessarily true. There could just be delay from the server or network.

We need to implement this by hitting the logs api endpoint and using the query params to tell it to not tail.

There are a few related issues (#1083, #1756) with logs, and I think this will be implemented as part of one of those issues.

@dnephin dnephin closed this Oct 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants