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

Error when no layout is present. #294

Merged
merged 3 commits into from
Jul 24, 2018
Merged

Error when no layout is present. #294

merged 3 commits into from
Jul 24, 2018

Conversation

rmarren1
Copy link
Contributor

@rmarren1 rmarren1 commented Jul 17, 2018 •
edited
Loading

Fixes #220, #287

@rmarren1
Copy link
Contributor Author

Did the pylint tests change for python3? Never had those circleci failures before.

@chriddyp chriddyp mentioned this pull request Jul 18, 2018
Merged
@chriddyp
Copy link
Member

💃 once tests pass

@valentijnnieman
Copy link
Contributor

💃 Is this the same reason #286 (comment) the tests are failing?

@rmarren1
Copy link
Contributor Author

I think so, they were all pylint errors. I'll just make the fixes it reports or add ignores

T4rk1n
T4rk1n reviewed Jul 18, 2018
View reviewed changes
dash/dash.py
'The layout was `None` '
'at the time that `run_server` was called. '
'Make sure to set the `layout` attribute of your application '
'before running the server.')
Copy link
Contributor

Choose a reason for hiding this comment

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

@ned2 Brought a good point in #286, anything in run_server won't be run by wsgi since it's called in if __name__ == '__main__':. Should move that check to _setup_server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, looks like it will also work in _setup_server

@rmarren1
Copy link
Contributor Author

I moved the check to _setup_server for wsgi support, and froze pylint==1.9.2 as @T4rk1n did in #286 to fix those testing errors

@rmarren1
Copy link
Contributor Author

I don't have the ability to merge on this repo. Can someone merge / give me permission?

@chriddyp chriddyp added the dash-meta-sponsored Work items whose development has been sponsored by a commercial partner https://plot.ly/dash/pricing label Jul 23, 2018
@chriddyp
Copy link
Member

I don't have the ability to merge on this repo. Can someone merge / give me permission?

@rmarren1 - I finally tracked this one down. Could you try now?

@rmarren1 rmarren1 merged commit ca570d7 into plotly:master Jul 24, 2018
Dash - A Pleasant and Productive Developer Experience automation moved this from In progress to Done Jul 24, 2018
@rmarren1 rmarren1 mentioned this pull request Aug 14, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@T4rk1n T4rk1n T4rk1n left review comments

Assignees
No one assigned
Labels
dash-meta-sponsored Work items whose development has been sponsored by a commercial partner https://plot.ly/dash/pricing
Projects
Dash - A Pleasant and Productive Deve...
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@rmarren1 @chriddyp @valentijnnieman @T4rk1n