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-deploy-env-override #5642

Merged
merged 3 commits into from
Aug 14, 2020
Merged

remove-deploy-env-override #5642

merged 3 commits into from
Aug 14, 2020

Conversation

lk86
Copy link
Contributor

@lk86 lk86 commented Aug 13, 2020

Remove forced DOCKER_DEPLOY_ENV from develop as it does not match the current setup. See https://buildkite.com/o-1-labs-2/coda/builds/1140#21325d9f-92c2-489e-9780-0f2b25075bf5 for how the bug manifests itself. At some point we should maybe use a different file/artifact name for the DEPLOY_ENV override but for the most part it already does the right thing (builds from buildkite/DOCKER_DEPLOY_ENV if it exists instead of using the one generated by build-artifact).

Thank you for contributing to Coda! Please see CONTRIBUTING.md if you haven't
yet. In that doc, there are more details around how to start our CI.

Explain your changes here.

Explain how you tested your changes here.

Checklist:

  • Document code purpose, how to use it
    • Mention expected invariants, implicit constraints
  • Tests were added for the new behavior
    • Document test purpose, significance of failures
    • Test names should reflect their purpose
  • All tests pass (CI will check this if you didn't)
  • Serialized types are in stable-versioned modules
  • Does this close issues? List them:

Closes #0000
Closes #0000

@lk86 lk86 requested a review from a team as a code owner August 13, 2020 22:15
@lk86 lk86 added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label Aug 14, 2020
@O1ahmad
Copy link
Contributor

O1ahmad commented Aug 14, 2020

Cool, all for the removal of this env file from under VCS & under buildkite\ though not sure what bug you're referencing @lk86.

I mean, the GitTriggeredDockerRelease is only meant to be triggered if the DOCKER_DEPLOY_ENV file changes which, based on your change, it shouldn't have unless I'm missing something. Notice in the Monorepo Triage step:

Triggering GitTriggeredDockerRelease for reason:

buildkite/DOCKER_DEPLOY_ENV

also notice how other jobs are triggered w/o reference to the merged changelist:

-

-- Running: ./buildkite/scripts/generate-diff.sh > _computed_diff.txt --

--

Triggering ArchiveNode for reason:

Makefile

buildkite/src/Jobs/ArchiveNode.dhall

src/lib/auxiliary_database/dune

src/lib/auxiliary_database/external_transition_database.ml

src/lib/auxiliary_database/external_transition_database.mli

src/lib/auxiliary_database/filtered_external_transition.ml

Seems like something is off with the diff generation or changelist compilation logic (re: the bug)...

@O1ahmad
Copy link
Contributor

O1ahmad commented Aug 14, 2020

Also, note that the override is ONLY if a DOCKER_DEPLOY_ENV is placed at (coda)repo ROOT, which is where the build/release logic checks. The buildkite/DOCKER_DEPLOY_ENV shouldn't affect overrides and was only meant to serve as an example/test for the flow in addition to potentially keeping tracking of deploys via VCS if the team thought it could help.

@lk86
Copy link
Contributor Author

lk86 commented Aug 14, 2020

This ends up more as a workaround for making manual builds of develop work properly, but as you mentioned the real bug is how the diff generation works (especially for manual develop builds) but fixing / improving that is harder than just not using this override in develop. Afaict everything is working properly with GitTriggeredDockerRelease on other branches / in all other scenarios so nothing there should really change, but until we fix the root cause (diffs in develop) I'd like to just stay away from committing DOCKER_DEPLOY_ENVs to develop.

@lk86 lk86 merged commit a3b7769 into develop Aug 14, 2020
@lk86 lk86 deleted the remove-deploy-env-override branch August 14, 2020 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants