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: Add RecentLogs support #1237

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

ZPascal
Copy link

@ZPascal ZPascal commented Aug 1, 2024

fix: #1181 & #1230

TODO:

  • Test the change
  • Add unit tests

@pivotal-david-osullivan
Copy link
Contributor

Thank you for the PR!

Compilation is currently failing on the implementation missing the new recentLogs method that you added to the client

ZPascal and others added 6 commits November 1, 2024 12:26
* because of the findFirst() on the envelopes, it could be type OUT or ERR, so we don't really care, as long as the payload is the same.
Also, we don't care about the precise argument to the recentLogs call
@ZPascal
Copy link
Author

ZPascal commented Nov 1, 2024

Thank you @pivotal-david-osullivan & @anthonydahanne for fixing the tests and the reactor part. I am also working on the issue again from the SAP side and I plan to set up a BBL environment to test the change in a CF environment. Let's sync on Slack how we can handle it together and we can discuss the test setup/steps.

@anthonydahanne
Copy link
Contributor

Thank you @pivotal-david-osullivan & @anthonydahanne for fixing the tests and the reactor part. I am also working on the issue again from the SAP side and I plan to set up a BBL environment to test the change in a CF environment. Let's sync on Slack how we can handle it together and we can discuss the test setup/steps.

cool!
the current struggle is supporting recent , since there does not seem to be a Websocket or SSE endpoint for streaming logs; apparently the CLI polls, so we'd need to do just that.

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