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

Add a configurable BASE_URL to allow Amundsen to be served outside of the root URL #435

Closed

Conversation

dmateusp
Copy link

Summary of Changes

Full description of the issue: amundsen-io/amundsen#424

On the Python (Flask) side, I leveraged url_for in index.html, a BASE_URL configuration variable and a middleware class. I also took the routes defined in amundsen_application/api/__init__.py and standardized them by using the same Blueprints mechanism as the rest was using.

On the Javascript side, I added an importable axios instance to inject the BASE_URL into the calls. Note that I have very little experience on front-end, I got some help from @rebeccarich at Earnest but I realized there's many things I don't understand there. For example I try to get the environment variable here, but I imagine it's not getting picked up because it's client-side ? Any feedback on which way we should do this?

On the scss side, I'm injecting the variable, but in this case it's done at compile time as far as I understand, which is undesirable (we probably don't want people building Amundsen themselves everytime they need BASE_URL to be set), also not sure about the right way of doing it

Happy to get initial feedback

Tests

Updates the tests to use the new axios

Documentation

TODO: add docs

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • PR includes a summary of changes, including screenshots of any UI changes.
  • PR adds unit tests, updates existing unit tests, OR documents why no test additions or modifications are needed.
  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public python functions and the classes in the PR contain docstrings that explain what it does
  • PR passes all tests documented in the developer guide

@stale
Copy link

stale bot commented May 14, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale stalebot believes this issue/PR is no longer active label May 14, 2020
@feng-tao feng-tao added the keep fresh Disables stalebot from closing an issue label May 19, 2020
@stale stale bot removed the stale stalebot believes this issue/PR is no longer active label May 19, 2020
@ttannis ttannis self-assigned this Jun 1, 2020
@ttannis
Copy link
Contributor

ttannis commented Aug 21, 2020

@dmateusp please let us know if you intend to move forward with this PR. Thanks!

@dmateusp
Copy link
Author

hey @ttannis I'm going to close this PR for now

@dmateusp dmateusp closed this Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep fresh Disables stalebot from closing an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants