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

fix: get server for Openapi specification #236

Merged
merged 17 commits into from
Feb 7, 2024
Merged

fix: get server for Openapi specification #236

merged 17 commits into from
Feb 7, 2024

Conversation

bkioshn
Copy link
Contributor

@bkioshn bkioshn commented Feb 5, 2024

Description

Get list of server for Openapi specification

Related Issue(s)

Closes #143

Description of Changes

  • The following is now added to servers in OpenAPI specification.
    • Local hostname
    • Local IPv4 and IPv6 address
  • Port is added to the back of the url instead of in a variables as it should be due to poem limitation
    https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.0.md#server-object
  • http-auto-server and https-auto-server, flags are added in case a prefix http//:, https//:, or both are needed and server_name
  • SERVER_NAME, HTTP_AUTO_SERVERS, HTTPS_AUTO_SERVERS, and ADDRESS can be set through environment variable

Related Pull Requests

#208

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@bkioshn bkioshn added documentation Pull requests that update a dependency file. enhancement New feature or request labels Feb 5, 2024
@bkioshn bkioshn self-assigned this Feb 5, 2024
@bkioshn bkioshn added the review me PR is ready for review label Feb 5, 2024
@bkioshn bkioshn requested a review from apskhem February 6, 2024 06:10
Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

Let add two env vars with defaults.

HTTP_AUTO_SERVERS=false
HTTPS_AUTO_SERVERS=true

and create one list for each if it's true

add another env var:

SERVER_NAME="https://www.someserver.com:1234"

@bkioshn bkioshn removed the review me PR is ready for review label Feb 7, 2024
@bkioshn bkioshn requested a review from stevenj February 7, 2024 09:42
Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

It would be ideal if all these could be both cli parameters or env vars.
The reason is, cli parameters are easier to deal with in local testing.
But env vars are easier to deal with in deployments where we don't want to change the containers.

catalyst-gateway/bin/src/settings.rs Show resolved Hide resolved
catalyst-gateway/bin/src/settings.rs Show resolved Hide resolved
catalyst-gateway/bin/src/settings.rs Show resolved Hide resolved
catalyst-gateway/bin/src/settings.rs Show resolved Hide resolved
@bkioshn bkioshn requested a review from stevenj February 7, 2024 11:46
Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

LGTM

@bkioshn bkioshn merged commit efb9c13 into main Feb 7, 2024
22 checks passed
@bkioshn bkioshn deleted the fix/openapi-server branch February 7, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Pull requests that update a dependency file. enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

🛠️ [TASK] : Add OpenAPI linting to our CI process.
3 participants