-
Notifications
You must be signed in to change notification settings - Fork 9
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
more changes for JBROWSE and MANET server settings #22
Conversation
I forgot to add the link that I am using as a reference: PatrickJS/PatrickJS-starter#386 |
@@ -24,14 +24,15 @@ class TranscriptViewer extends Component { | |||
// let externalPrefix = 'http://bw.scottcain.net/jbrowse/?data=data%2F'; | |||
// let externalPrefix = 'http://ec2-34-208-22-23.us-west-2.compute.amazonaws.com/jbrowse/overview.html?data=data%2F'; | |||
// let externalPrefix = 'http://localhost/jbrowse/overview.html?data=data%2F'; | |||
let externalPrefix = 'http://jbrowse.alliancegenome.org/jbrowse/overview.html?data=data%2F'; | |||
// let externalPrefix = 'http://jbrowse.alliancegenome.org/jbrowse/overview.html?data=data%2F'; | |||
let externalPrefix = this._http.get(process.env.JBROWSE_URL) + '/jbrowse/overview.html?data=data%2F'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite sure what this _http
is supposed to be. When I run this branch and go to a gene page I get the error Cannot read property 'get' of undefined
. Can it simply be
let externalPrefix = process.env.JBROWSE_URL + '/jbrowse/overview.html?data=data%2F';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Patrick, Sorry I had these small changes last Friday but didn't push because the site wasn't working well enough for me to test. I hope it is now. basically the _http came from the example - definitely shouldn't be there. I hope this results in the env variables forming the correct urls / ports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paaatrick I ended up having a weird error with commits on this repo that I have never had before. I am not sure if it had something to do with running it in docker... I hope not. Anyways I had to make a new clone and apply the changes. @sibyl229 helped me with the configuration - I was running in somethings that I wouldn't have if I was more familiar with the config file. Nothing major.
In the way of testing, I was printing out the environment variables in a template and could see the environment variable in the UI. Let me know if you have an further changes that you would like. I believe this will help @oblodgett lineup GOCD configuration to point at a specific Manet or JBrowse ip:port for the environment the UI is being deployed in. Cheeers
I have to consult with @scottcain and @oblodgett to know exactly what is needed here. What i am, theoretically, doing is passing environment variables into the JavaScript bundle for URLs and ports for JBrowse and Manet. Once I got started with putting things into the src I discovered that the code didn't have a MANET URL... Might not be necessary. Might also want to put environment variables in the Dockerfile. i have been developing this from within the container.
I was not able to tests, yet, because currently dev.alliancegenome.org is down. Will start in again when things are up and running on the backend.
This pull request still needs a little work but should be most of the way there.