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.

Sign up for GitHub

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

Jump to bottom

Setup improvements - Preflight #1473

Merged
merged 12 commits into from
Aug 20, 2020
Merged

Setup improvements - Preflight #1473

merged 12 commits into from
Aug 20, 2020

Conversation

paolodamico
Copy link
Contributor

@paolodamico paolodamico commented Aug 19, 2020 •
edited
Loading

Changes

  • Introduces the preflight check screen for self-hosted installations (see screen recordings below).
  • Allows the user to select installation usage for experimentation or production use (and TLS requirements are changed accordingly).
  • Introduces API endpoint /_preflight/ which returns a JSON response indicating whether Django server, Redis & the database are available and ready for usage.

Additional notes:

  • The preflight check is run again by completely refreshing the webpage to reload SSL/TLS protocol (if available).
    • The state is stored on local storage to reload where the user left off.
  • /preflight is currently the only route from the frontend that is allowed to be accessed without being authenticated (special handling on both the backend and frontend required).
  • First stage of RFC: Improved setup experience #1408

Experimentation mode

Live mode

Checklist

  • N/A. All querysets/queries filter by Team (if this PR affects any querysets/queries)
  • Backend tests (if this PR affects the backend)
  • Cypress E2E tests (if this PR affects the front and/or backend)

@timgl timgl temporarily deployed to posthog-feat-setup-impr-njixrl August 19, 2020 17:15 Inactive
@timgl timgl temporarily deployed to posthog-feat-setup-impr-njixrl August 19, 2020 17:51 Inactive
@timgl timgl temporarily deployed to posthog-feat-setup-impr-njixrl August 19, 2020 17:56 Inactive
@timgl timgl temporarily deployed to posthog-feat-setup-impr-njixrl August 19, 2020 18:01 Inactive
@timgl timgl temporarily deployed to posthog-feat-setup-impr-njixrl August 19, 2020 23:51 Inactive
@timgl timgl temporarily deployed to posthog-feat-setup-impr-njixrl August 20, 2020 01:55 Inactive
@paolodamico paolodamico marked this pull request as ready for review August 20, 2020 01:57
@paolodamico paolodamico changed the title [WIP] Setup improvements Setup improvements - Preflight Aug 20, 2020
ungless
ungless reviewed Aug 20, 2020
View reviewed changes
Copy link
Contributor

@ungless ungless left a comment

Choose a reason for hiding this comment

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

I rate it! Maybe also we should check if SECRET_KEY is left default if they choose that this is a production instance? Not sure about the security implications of this.

@Twixes
Copy link
Collaborator

Twixes commented Aug 20, 2020 •
edited
Loading

I think "running in DEBUG mode!" should be discouraging enough.

@ungless
Copy link
Contributor

ungless commented Aug 20, 2020

@Twixes my understanding is that many users forget to change the default secret key and this could potentially catch quite a few of those users if we are explicit about it.

Twixes
Twixes reviewed Aug 20, 2020
View reviewed changes
Copy link
Collaborator

@Twixes Twixes left a comment

Choose a reason for hiding this comment

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

Just some UX thoughts.

frontend/src/scenes/setup/PreflightCheck.js Outdated Show resolved Hide resolved
frontend/src/scenes/setup/PreflightCheck.js Outdated Show resolved Hide resolved
frontend/src/scenes/setup/PreflightCheck.js Outdated Show resolved Hide resolved
frontend/src/scenes/setup/PreflightCheck.js Outdated Show resolved Hide resolved
frontend/src/scenes/setup/PreflightCheck.js Outdated Show resolved Hide resolved
frontend/src/scenes/setup/PreflightCheck.js Outdated Show resolved Hide resolved
frontend/src/scenes/setup/PreflightCheck.js Outdated Show resolved Hide resolved
frontend/src/scenes/setup/PreflightCheck.js Outdated Show resolved Hide resolved
frontend/src/scenes/setup/PreflightCheck.js Show resolved Hide resolved
@Twixes
Copy link
Collaborator

Twixes commented Aug 20, 2020

@ungless Non-DEBUG instances now fail on start attempt if SECRET_KEY is set to the default, so that should cover it.

@ungless
Copy link
Contributor

ungless commented Aug 20, 2020

@Twixes I see! That works great then

@timgl timgl temporarily deployed to posthog-feat-setup-impr-njixrl August 20, 2020 20:41 Inactive
@paolodamico
Copy link
Contributor Author

Thans for the thorough review @Twixes & @ungless! Feel free to review again with the new changes, otherwise I'll merge tomorrow morning PT.

@Twixes Twixes temporarily deployed to posthog-feat-setup-impr-njixrl August 20, 2020 21:29 Inactive
@Twixes
Copy link
Collaborator

Twixes commented Aug 20, 2020 •
edited
Loading

I made some UI changes, take a look @paolodamico and say if anything's out of place. Definitely better on mobile now, though still not perfect.

@paolodamico paolodamico merged commit 7605d7a into master Aug 20, 2020
@paolodamico paolodamico deleted the feat/setup-improvements-1 branch August 20, 2020 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Twixes Twixes Twixes left review comments

@ungless ungless ungless left review comments

@timgl timgl Awaiting requested review from timgl

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@paolodamico @Twixes @ungless @timgl