[Bug 1473091] Migrate mozilla/redash to circleci 2.0 #488
Conversation
haroldwoo
force-pushed
the
redash-circle2
branch
from
135e176
to
dfb691f
Compare
|
This looks good to me. Upstream has a PR for CircleCI 2.0 support getredash#2672 which includes a lot more integration tests. I think once upstream is merged we should pull those tests into this config. We probably still need to maintain a different CircleCI configuration due to our deployment model. CCing @jezdez for input. |
|
How often do we merge with upstream? It does make sense to pull those tests into here once we do, but we can cross that bridge later? |
.circleci/config.yml
Outdated
| only: /^m[0-9]+(\.[0-9]+)?$/ | ||
| branches: | ||
| only: | ||
| # Do we only care about milestone tags on these branches? |
There was a problem hiding this comment.
i'm not sure what branches we care about deploying milestone tags?
There was a problem hiding this comment.
Tags are just pointers to a commit and have nothing to do with branches. I think the branches are supposed to be set to ignore based on my reading here.
haroldwoo
force-pushed
the
redash-circle2
branch
from
d827e6a
to
c9d0cbc
Compare
|
@jasonthomas Oh dang, this seems like some overlap here, since @jrbenny35 has been working on this in #439 as well. I don't mind which solution is chosen and I have way too little experience with Circle 2.0 to make the call. But we need to make sure that the solution here is not completely different from what will soon (given the deadline) land upstream. Upstream there is still a question whether the redash-ui-tests (that @jrbenny35 and @hackebrot have created on our side) should be included upstream (my gut says no since the tests aren't ready yet and we haven't full buy-in from upstream, but I hope to have the main maintainer reply to our question about it). @haroldwoo @jasonthomas Could you talk with @jrbenny35 about how we can find a common denominator between the efforts and solve the issue for both our fork and upstream? We strive to reduce the divergence from upstream as much as possible given our current work of porting our changes upstream after all and eventually plan to have little or no modification in a fork (dream scenario: we deploy from a separate repo that just contains inherited Docker info and extended Circle config). |
|
Thank you all for your effort of course, much appreciated! |
|
The purpose of this PR was simply to upgrade our existing circle code from 1.0 to 2.0 before 1.0 deprecation. When/If the other PR lands upstream, we can consolidate our circle builds then. Does this sound fair? |
|
@haroldwoo Be that as it may, merging this will create more work than to just find a common ground since @jrbenny35 is a Mozilla test engineer working on this anyway. @jrbenny35 Do you think you can look through this and see if this is compatible with your approach? |
|
Disregard what I said in my last comment, I spoke with @haroldwoo more about this on IRC and he suggested to merge this now and then wait for the outcome from the upstream PR (getredash#2672) and deal with the result when merging upstream into our fork at the next occasion. |
.circleci/config.yml
Outdated
| printf '{"commit":"%s","version":"%s","source":"https://github.com/%s/%s","build":"%s"}\n' "$CIRCLE_SHA1" "$CIRCLE_TAG" "$CIRCLE_PROJECT_USERNAME" "$CIRCLE_PROJECT_REPONAME" "$CIRCLE_BUILD_URL" > version.json | ||
| - run: | ||
| command: docker build -t app:build . | ||
| no_output_timeout: 20m |
There was a problem hiding this comment.
I'm trying to understand the purpose of the build job and can't find an equivalent in the old config. It's run for every tag and all branches except gh-pages IIUC. Is this just to make sure the image build succeeds?
Could we remove the writing of the version.json since it was abstracted away in the bin/deploy script for the other jobs?
There was a problem hiding this comment.
Yes it's just to ensure that the image builds properly. I will remove the version.json for the build step
.circleci/config.yml
Outdated
| else | ||
| # Deploy a release tag... | ||
| ./bin/deploy "rc" | ||
| fi |
There was a problem hiding this comment.
Could we split the deploy job into two jobs deploy-master and deploy-rc (and update the workflows below) instead of having the shell script with the condition? That would make it more obvious in the Circle UI what's happening.
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Disable test that tests the login page and the existence of the remote auth link there. We override this functionality for user experience reasons via redash-stmo and redirect from the login page to the remote auth URL. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Disable test that tests the login page and the existence of the remote auth link there. We override this functionality for user experience reasons via redash-stmo and redirect from the login page to the remote auth URL. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Disable test that tests the login page and the existence of the remote auth link there. We override this functionality for user experience reasons via redash-stmo and redirect from the login page to the remote auth URL. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Disable handlers test that tests the login page and the existence of the remote auth link there. We override this functionality for user experience reasons via redash-stmo and redirect from the login page to the remote auth URL. Update metrics test since our auth changes add a database call which also gets timed, and therefore assert this was called only once fails. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Disable handlers test that tests the login page and the existence of the remote auth link there. We override this functionality for user experience reasons via redash-stmo and redirect from the login page to the remote auth URL. Update metrics test since our auth changes add a database call which also gets timed, and therefore assert this was called only once fails. Refs #13 Refs #37
- Use new master / rc release release strategy (#440) - Migrate Circle CI 2.0 (#488, #502, #923) - Install redash-stmo. In the long run we'll be able to install additional dependencies by having an own Dockerfile to build images based on the Redash image but that installs additional Python dependencies. But until we have a fork with lots of changes ourselves we need to do it this way. Redash-stmo contains the ability to hook up our own Dockerflow library. Disable handlers test that tests the login page and the existence of the remote auth link there. We override this functionality for user experience reasons via redash-stmo and redirect from the login page to the remote auth URL. Update metrics test since our auth changes add a database call which also gets timed, and therefore assert this was called only once fails. Refs #13 Refs #37
No description provided.