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

chore: Remove logging.yaml #1809

Merged
merged 2 commits into from
Aug 28, 2024
Merged

Conversation

dpaasman00
Copy link
Contributor

@dpaasman00 dpaasman00 commented Aug 20, 2024

Proposed Change

Cleanup references to logging.yaml

Checklist
  • Changes are tested
  • CI has passed

Copy link
Contributor

@BinaryFissionGames BinaryFissionGames left a comment

Choose a reason for hiding this comment

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

Seems like we could also remove the config/logging.* files

@dpaasman00 dpaasman00 force-pushed the chore/remove-logging.yaml branch from ac92663 to a8bdd95 Compare August 26, 2024 14:55
@dpaasman00 dpaasman00 marked this pull request as ready for review August 26, 2024 14:59
@dpaasman00 dpaasman00 requested a review from a team as a code owner August 26, 2024 14:59
@dpaasman00 dpaasman00 requested review from jsirianni and justinianvoss22 and removed request for justinianvoss22 August 26, 2024 14:59
@@ -263,7 +252,6 @@ dockers:
extra_files:
- release_deps/VERSION.txt
- release_deps/plugins
- config/logging.stdout.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Will containers continue to log to stdout / error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unsure about containers. All I know is the supervisor starts the agent as it's own subprocess and directs output from stdout to a file called agent.log. I'm not sure what that means for containers...

Relevant code

Copy link
Member

Choose a reason for hiding this comment

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

I think this should be a blocker that we resolve before cutting over to v2. Logging to a file within the container will be a no go.

To be clear, it does not need to block this PR, but we do need to track this as an issue.

.goreleaser.yml Show resolved Hide resolved
.goreleaser.yml Show resolved Hide resolved
@dpaasman00 dpaasman00 requested a review from jsirianni August 28, 2024 11:17
Copy link
Member

@jsirianni jsirianni left a comment

Choose a reason for hiding this comment

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

Dakota and I spoke offline about supporting container stdout / stderr logging. The tentative plan is to push for Supervisor support in this area, probably by capturing agent output and writing it to its own stdout / stderr.

@dpaasman00 dpaasman00 merged commit e62087d into release/v2.0.0 Aug 28, 2024
16 checks passed
@dpaasman00 dpaasman00 deleted the chore/remove-logging.yaml branch August 28, 2024 13:25
dpaasman00 added a commit that referenced this pull request Sep 11, 2024
* remove logging.yaml

* remove logging files
dpaasman00 added a commit that referenced this pull request Oct 7, 2024
* remove logging.yaml

* remove logging files
dpaasman00 added a commit that referenced this pull request Nov 14, 2024
* remove logging.yaml

* remove logging files
dpaasman00 added a commit that referenced this pull request Dec 9, 2024
* remove logging.yaml

* remove logging files
dpaasman00 added a commit that referenced this pull request Dec 16, 2024
* remove logging.yaml

* remove logging files
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