Skip to content

Comments

add af3 deployment logs support#1878

Merged
jaketf merged 5 commits intomainfrom
af3-logs
Jun 26, 2025
Merged

add af3 deployment logs support#1878
jaketf merged 5 commits intomainfrom
af3-logs

Conversation

@jaketf
Copy link
Contributor

@jaketf jaketf commented Jun 24, 2025

Description

Describe the purpose of this pull request.

  • removes AF3 block for astro deployment logs
  • introduces logic to get the apiserver component if AF3 else webserver component

🎟 Issue(s)

fixes #1877

🧪 Functional Testing

https://astronomer.slack.com/archives/C07FWJXBQ3X/p1750789166767639

📸 Screenshots

Add screenshots to illustrate the validity of these changes.

📋 Checklist

  • Rebased from the main (or release if patching) branch (before testing)
  • Ran make test before taking out of draft
  • Ran make lint before taking out of draft
  • Added/updated applicable tests
  • Tested against Astro-API (if necessary).
  • Tested against Houston-API and Astronomer (if necessary).
  • Communicated to/tagged owners of respective clients potentially impacted by these changes.
  • Updated any related documentation

Copy link
Contributor

@ianbuss ianbuss left a comment

Choose a reason for hiding this comment

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

Couple of small comments.

go.mod Outdated
go 1.23.0

// toolchain go1.24.1
toolchain go1.24.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this toolchain entry? Should we just update the go version to 1.24?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good question.
I have issues when this line is present when using debugger in IDE.
I have had issues in the past when this line is missing when running make fmt (gofumpt under the hood).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll remove it for now and see if anything breaks in CI

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't consider it blocking but it would be nice for the values to agree I guess? Anyway, approving as it is commented in main.

Copy link
Contributor

@ianbuss ianbuss left a comment

Choose a reason for hiding this comment

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

LGTM

@jaketf jaketf merged commit fcbc70c into main Jun 26, 2025
4 of 5 checks passed
@jaketf jaketf deleted the af3-logs branch June 26, 2025 22:05
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.

support astro deployment logs for AF3

2 participants