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

Remove hardcoded references to rucio-ui-dev.cern.ch #5171

Closed
dchristidis opened this issue Jan 21, 2022 · 9 comments · Fixed by #5301
Closed

Remove hardcoded references to rucio-ui-dev.cern.ch #5171

dchristidis opened this issue Jan 21, 2022 · 9 comments · Fixed by #5301
Assignees
Milestone

Comments

@dchristidis
Copy link
Contributor

Motivation

It is expected that this node will cease to exist very soon.

Modification

$ git grep rucio-ui-dev.cern.ch
lib/rucio/web/ui/static/base.js:    if (window.location.host == "rucio-ui-dev.cern.ch") {
lib/rucio/web/ui/static/http_api_usage.js:  var src = "https://rucio-ui-dev.cern.ch/kibana/#/dashboard/Rucio-RESTAPI-Account-Usage?embed&_g=("+
@nimishbongale
Copy link
Member

nimishbongale commented Feb 20, 2022

Can I take this up @maany @dchristidis @bari12?

If so, do we want it to be extracted into a config file and then be replaced eventually or removed entirely?

@bari12
Copy link
Member

bari12 commented Feb 22, 2022

Sorry for the late reply, yes please go ahead 👍

@nimishbongale
Copy link
Member

nimishbongale commented Feb 22, 2022

yes please go ahead 👍

Thank you! Just to confirm, I can extract this URL string out into maybe a config.json file and refer it from there? Would that be the desired way of implementation?

@maany
Copy link
Member

maany commented Feb 22, 2022

Maybe an environment variable is better in this case ( to avoid creating a file with a single configuration variable). To completely resolve this issue, we'd also have to update the Helm Chart for the UI to allow people to provide this value to the UI containers and provide the documentation to the users so they are aware of this ;) .

@nimishbongale
Copy link
Member

Maybe an environment variable is better in this case ( to avoid creating a file with a single configuration variable). To completely resolve this issue, we'd also have to update the Helm Chart for the UI to allow people to provide this value to the UI containers and provide the documentation to the users so they are aware of this ;) .

Thanks for this, will get to work right away! 👍

@nimishbongale
Copy link
Member

nimishbongale commented Feb 26, 2022

Maybe an environment variable is better in this case (to avoid creating a file with a single configuration variable)

Passing environment variables into static .js files seem a little more complicated than I had expected. The environment variable would be accessible only to my flask server, which I would have to pass into my HTML templates, via render_template, and then pass it as a query parameter into the static .js files, and access it accordingly. Is there a better way? Let me know 😄

@maany
Copy link
Member

maany commented Feb 28, 2022

One way to get around this, would be to modify the docker entrypoint for the UI containers

The script should check if the environment variable for hostname is defined i.e. RUCIO_HOST. If found, it should modify the concerned js files using the sed utility.

@bari12 what do you think? also, do you agree that we can use a new environment variable called RUCIO_HOST or should we call it something else?

@nimishbongale
Copy link
Member

docker entrypoint for the UI containers

Definitely a better way to solve this problem. Thanks for this! 👍

@bari12
Copy link
Member

bari12 commented Feb 28, 2022

Sorry getting a bit late into this. I think you can just remove these entirely. No need to have this configurable etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants