-
Notifications
You must be signed in to change notification settings - Fork 20
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
Create anvilprod deployment and build on GitLab #2804
Conversation
@NoopDog Please review today. |
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 @hannes-ucsc I don't have any comment on the .gitlab section but have requested you revert the change to the header config. I would like to suggest we update the config for https://prod.anvil.gi.ucsc.edu/ on a branch and not on main and merge into main once the site goes live on anvilproject.org.
src/config/anvil/config.ts
Outdated
@@ -26,7 +26,7 @@ const config: SiteConfig = { | |||
}, | |||
{ | |||
label: "Datasets", | |||
url: `${getDatasetsEnvironmentUrl()}data`, | |||
url: "/explore/datasets", |
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.
@hannes-ucsc can you please revert this change? Thanks! D
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.
I anticipated this change being controversial. I didn't understand getDatasetsEnvironmentUrl
so I just clobbered it with the hard-coded line. Is there any way this could be made dynamic so that we don't have to maintain a branch for this one-line difference? We have a way to inject environment variables into the build. See my next comment.
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.
We are in the process of implementing the catalog/explorer selector now, so how about for right now, we make this PR deal with the prod build and deploy only, and we remove the setting of the value for the "Datasets" link as this will change soon anyhow.
We want to keep the catalog/explorer selector on its own branch until we are ready to deploy this to anvilproject.org, which I believe is in several weeks at the earliest. We will need to do deployments until the switchover, so I don't want to make changes to main
that change the functionality for the managed access deployment only.
If I have the schedule wrong, let me know.
cc: @bvizzier-ucsc
@@ -0,0 +1,3 @@ | |||
variables: |
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.
This is where the environment variables are set.
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.
@hannes-ucsc, @bvizzier-ucsc You need to do the equivalent of export GATSBY_ENV="ANVIL-PROD"
before running the build.
See:
Then you can remove the clobbered line.
Cheers,
Dave
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.
I removed the commit.
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.
We are setting GATSBY_ENV in base.yaml below.
Then you can remove the clobbered line.
That surprises me since the clobbered line always appends data
which wouldn't work in this case.
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.
But again, I removed the commit and will let you figure this out.
@NoopDog, I removed the problematic commit. I think you are in a better position to implement the equivalent functionality i.e., the backlink from portal to browser. I propose that you review this, and assuming you approve, merge it into the We are also open to using a different branch to track the work on prod.anvil.gi.ucsc.edu as long as that branch is never force-pushed to. @achave11-ucsc knows how to take changes from this repository and deploy them to prod.anvil.gi.ucsc.edu. Please let him know what the branch name is, and when that branch contains the equivalent to the commit I removed. He can then take the necessary steps to deploy that branch to prod.anvil.gi.ucsc.edu. |
I added a commit that adds a build for the consortia site. You can preview this here: https://anvil.gi.ucsc.edu/consortia Not being an expert in this code base, I followed the same approach as before and did not bother with fixing any links in the top nav of the site. The deployment part is in place, the rest is configuration, which I'll leave to you. |
The corresponding Azul PR is |
Not required |
Same as DataBiosphere#1 in UCSC's fork, but for the upstream repository.