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

Update GitHub Actions and Set the Platform Requirement to macOS 11 #374

Merged
merged 68 commits into from
Jan 3, 2022

Conversation

PSchmiedmayer
Copy link
Contributor

@PSchmiedmayer PSchmiedmayer commented Nov 4, 2021

Update GitHub Actions and Set the Platform Requirement to macOS 11

♻️ Current situation & Problem

Currently, GitHub Action workflows and other configuration files are located in every repo. This PR moves them to the Apodini/.github repo.
In addition, the GitHub Actions do not include macOS builds due to the lack of backward compatibility of the concurrency features introduced in Swift 5.5 to older Apple OS versions.

💡 Proposed solution

The PR removes the local GitHub Action workflows and other configuration files and references the GitHub Action workflows and other configuration files in the Apodini/.github repo.
Xcode 13.2 introduced backward compatibility of, e.g., async/await to older Apple OS versions. The GitHub Actions in the Apodini/.github repo are updated to use the latest stable Swift versions on Linux and Xcode 13.2 on the GitHub macOS 11 build agents.
Also closes #397 and #396.

🛠 Current State

⚙️ Release Notes

  • Supports building for macOS 11

Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@PSchmiedmayer
Copy link
Contributor Author

PSchmiedmayer commented Dec 18, 2021

@moritzsternemann & @philippzagar Thanks for your input, I have updated to test cases to reflect your points and the current state of Swift NIO 👍

@lukaskollmer Also thanks to you for your input! I have further investigated the LocalHostDeploymentIssue. As far as I could investigate it, the HTTPClient is working as expected but the proxy web service is not starting and accepting requests as expected. I have added a few more logs in the tests logging more details about the HTTPClient errors. I also extended the timeout and manually send request to the proxy web services at e.g. http://localhost:80/v1 using curl and received a curl: (7) Failed to connect to localhost port 80: Connection refused message. So my guess is that even though the logs indicate that the proxy web service is starting there seems to be an issue accepting requests. Do you have an idea at what I should further look at and where this might come from?

@lukaskollmer
Copy link
Member

lukaskollmer commented Dec 18, 2021

@PSchmiedmayer I just had a look at this and I managed to fix it. Can I push onto this branch?

The issue seems to have been that ApodiniDeploy was unable to construct a HTTP request object for the AsyncHTTPClient for proxying a client request to the actual server (or rather the AsyncHTTPClient rejected ApodiniDeploy's request object) because the hostname and port were missing, because the HTTPRequest created by ApodiniNetworking to represent the client request contained a URL object where these fields were empty, which was the case because the HTTP request head (from which ApodiniNetworking constructs the URL) simply doesn't contain these kinds of information. So the change that caused this to become a crash was either in NIO changing the format of the request heads, or in the AsyncHTTPClient's changing its checking of url hostnames.

@PSchmiedmayer
Copy link
Contributor Author

@lukaskollmer Thanks for taking a look at this! That's interesting, feel free to push to this branch 🚀💪

@lukaskollmer
Copy link
Member

@PSchmiedmayer done. let me know if this didn't fix things

@PSchmiedmayer
Copy link
Contributor Author

@lukaskollmer Thanks a lot! I have adapted your changes a bit to use the hostname instead of the bind address and fixed a few assumptions here and there and all tests seem to pass now. There is still an issue with Swift Concurrency on macOS 11 but I think that Xcode 13.2.1 will fix this as indicated in its release notes, we will probably have to wait a bit until it arrives on the GitHub Actions runners but GitHub already seems to have started with adding it: actions/runner-images#4765

Copy link
Collaborator

@PaulsAutomationBot PaulsAutomationBot left a comment

Choose a reason for hiding this comment

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

🤖👍

@PSchmiedmayer PSchmiedmayer enabled auto-merge (squash) January 3, 2022 13:55
@PSchmiedmayer PSchmiedmayer merged commit 6480d42 into develop Jan 3, 2022
@PSchmiedmayer PSchmiedmayer deleted the feature/ciimprovements branch January 3, 2022 14: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.

[Security] Workflow release.yml is using vulnerable action softprops/action-gh-release
5 participants