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

DM-36868: Clean up the sqlproxy service #1812

Merged
merged 3 commits into from
Nov 8, 2022
Merged

DM-36868: Clean up the sqlproxy service #1812

merged 3 commits into from
Nov 8, 2022

Conversation

rra
Copy link
Member

@rra rra commented Nov 4, 2022

Rename this to sqlproxy-cross-project to match the science-platform name and overall Phalanx configuration. Add documentation.

Clean up the Helm chart configuration. Remove a bunch of unneeded variables and make the interface higher-level, roughly matching how Gafaelfawr's instance of this has been configured. Remove global variables that were never used.

Update the minikube and Kubernetes versions used for testing.

@rra rra marked this pull request as ready for review November 4, 2022 00:05
@rra rra force-pushed the tickets/DM-36868 branch 4 times, most recently from 25e895f to fc69faf Compare November 4, 2022 20:45
@rra
Copy link
Member Author

rra commented Nov 4, 2022

The docs build is failing because of a link to a page that is also created by this PR, so should resolve once it's merged.

@rra rra requested a review from athornton November 4, 2022 20:56
@rra rra force-pushed the tickets/DM-36868 branch from a331c67 to a8be3b3 Compare November 7, 2022 17:09
Copy link
Member

@athornton athornton left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me.

@@ -2,31 +2,28 @@
apiVersion: v1
kind: Namespace
metadata:
name: sqlproxy-cross-project
name: "sqlproxy-cross-project"
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why you're quoting all the strings in this file, when we don't elsewhere, but it seems unlikely to hurt anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've been adding quoting to each file in Phalanx when I touch it because it means one doesn't have to remember YAML's quoting rules, but I admit that I'm inconsistent about it. (A bunch of the applications have already been converted to fully quoted, though, so this is making them more consistent.)

rra added 2 commits November 8, 2022 11:43
Rename this to sqlproxy-cross-project to match the science-platform
name and overall Phalanx configuration.  Add documentation.

Clean up the Helm chart configuration.  Remove a bunch of unneeded
variables and make the interface higher-level, roughly matching how
Gafaelfawr's instance of this has been configured.  Remove global
variables that were never used.
@rra rra force-pushed the tickets/DM-36868 branch from a8be3b3 to 7eeeeb9 Compare November 8, 2022 19:43
Otherwise, we create a deadlock that causes the new docs to never
be published.
@rra rra force-pushed the tickets/DM-36868 branch from 9853595 to 5ad3069 Compare November 8, 2022 20:17
@rra rra merged commit 7a1e748 into master Nov 8, 2022
@rra rra deleted the tickets/DM-36868 branch November 8, 2022 21:01
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.

2 participants