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

Turns on verbose docker builds for make dev #4974

Merged

Conversation

zenmonkeykstop
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop commented Nov 7, 2019

Status

Ready for review

Description of Changes

Fixes #4972 .

Sets an env var, DOCKER_BUILD_VERBOSE required to make make dev verbose. Also bumps TBB_VERSION to 9.0.1, as 8.* versions are no longer available.

Testing

Run make dev, confirm that the docker build progress is displayed, and that dev container starts.

@zenmonkeykstop zenmonkeykstop changed the title Turns on verbose docker builds for make dev Turns on verbose docker builds for make dev Nov 7, 2019
@redshiftzero
Copy link
Contributor

heads up #4973 contains the TBB 9.0.1 change and tests are failing in CI, so we'll need to investigate what's happening there (not sure if anyone's tested to see if they can reproduce the test failure locally)

@zenmonkeykstop
Copy link
Contributor Author

@redshiftzero yup looks like an actual SD bug - in 9.0.1, the JI source filter widget isn't showing with default page settings. Taking a look.

@redshiftzero
Copy link
Contributor

Let's also mark test_admin_edits_hotp_secret with pytest.mark.skip? Seems to be hanging reliably in CI. For each one of these tests that we're xfailing or skipping we should file an issue and mark help wanted (good small thing for a new contributor to help investigate)

@redshiftzero
Copy link
Contributor

Did that, tests pass locally with that change, so let's see what CI thinks (running now)

redshiftzero
redshiftzero previously approved these changes Nov 8, 2019
Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we've xfailed one test and skipped two others, but this gets CI back to green to unblock other merges, approving and will open followups for these test updates which we should fix prior to the next release

@redshiftzero
Copy link
Contributor

@zenmonkeykstop or @emkll when you get a chance can you 👍 my changes here? (so I'm not approving my own commits on the end). @zenmonkeykstop's changes otherwise lgtm

@redshiftzero
Copy link
Contributor

I should note for the reviewer here: this doesn't address the second part of #4972 (comment) regarding supervisord in the dev env, so we'll need to open another followup for that (doing now and will cross-link)

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for opening #4977 #4978 to track / follow-up on these failures. Good to merge when CI passes.

@redshiftzero redshiftzero merged commit 8158c1b into freedomofpress:develop Nov 8, 2019
@deeplow
Copy link
Contributor

deeplow commented Nov 13, 2019

@zenmonkeykstop the docs (the tip in particular) refer users to also set DOCKER_BUILD_VERBOSE=true so they should also be changed.

I would propose the following be removed as it is now default behavior: If you are unsure about what happens, you can get a more verbose output by setting the environment variable export DOCKER_BUILD_VERBOSE=true..

If so, should I open a follow up issue and PR?

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

Successfully merging this pull request may close these issues.

make dev stuck at the beginning on Debian Buster
4 participants