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 a separate ENV variable for Authentication URL is supplied #393

Merged
merged 5 commits into from
Apr 12, 2019

Conversation

jason-fox
Copy link
Contributor

Related #392 - This PR offers an alternative AUTHENTICATION_URL which means it can be different to the ACCESS_TOKEN_URL

The fallback is to use FIWARE_IDM_SERVER as previously

@coveralls
Copy link

coveralls commented Mar 27, 2019

Coverage Status

Coverage decreased (-0.005%) to 78.94% when pulling 4867790 on jason-fox:feature/idm-url into 8418eac on Wirecloud:develop.

@aarranz
Copy link
Member

aarranz commented Mar 31, 2019

Hi @jason-fox,

Thank you very much for your contributions. I'm missing some minor update on the installation guide regarding this new setting (on the IdM section) as well as some basic test to cover new code lines.

Regarding your current changes, everything looks great except the name of the setting. I prefer something like FIWARE_IDM_PUBLIC_URL. What do you think? Also, it would be great if @mjimenezganan can provide his opinion about the setting name.

@aarranz
Copy link
Member

aarranz commented Apr 12, 2019

Hi @jason-fox, can you proceed an update the environment name?. Miguel is a bit busy to write here, but I've talked with him and he agrees.

@jason-fox
Copy link
Contributor Author

@aarranz - PR has been fixed according to review.

@aarranz
Copy link
Member

aarranz commented Apr 12, 2019

Merging, Thank you very much for the contribution!

@aarranz aarranz merged commit cd82f41 into Wirecloud:develop Apr 12, 2019
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.

3 participants