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

Use proper AUTH_URL values in next.config.js and AuthProvider #1954

Merged
merged 1 commit into from
Mar 16, 2021

Conversation

humphd
Copy link
Contributor

@humphd humphd commented Mar 16, 2021

@manekenpix and @HyperTHD noticed a few bugs in the way the staging auth fix was being done. This corrects the problems by properly defining the AUTH_URL for next's build, and using it in the AuthProvider.

@@ -28,7 +28,7 @@ API_VERSION=v1
# Auth Service
################################################################################

# Auth Service Port (default is 4444)
# Auth Service Port (default is 7777)
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't feed discovery use 7777?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It did, but I've now updated it. We had two servers on the same port (auth was always on 7777 so it wasn't available when Tony chose it).

birtony
birtony previously approved these changes Mar 16, 2021
Copy link
Contributor

@birtony birtony left a comment

Choose a reason for hiding this comment

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

@humphd said we should test this on master

@humphd
Copy link
Contributor Author

humphd commented Mar 16, 2021

Rebased to pickup the posts changes on master, I need two more reviews? I'd love to get this in soon.

Copy link
Contributor

@HyperTHD HyperTHD left a comment

Choose a reason for hiding this comment

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

We'll need to merge to see if this works, but I tested locally and it worked just fine

@HyperTHD HyperTHD merged commit 2bd63f1 into Seneca-CDOT:master Mar 16, 2021
@humphd
Copy link
Contributor Author

humphd commented Mar 16, 2021

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: deployment Production or Staging deployment area: docker area: nextjs Nextjs related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants