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

A demo endpoint to support onboarding of new team members #156

Closed
wants to merge 1 commit into from

Conversation

docsteveharris
Copy link
Contributor

@docsteveharris docsteveharris commented Nov 25, 2022

Simple demonstration of how to write both the API and the Plotly Dash web page.
I've accidentally wrapped up a typo fix to the hospital endpoint in this but otherwise all the changes are related to the demo endpoint or to the documentation. I also added a couple of notes on how to work with PyCharm since this was my first time trying this in anger.

@jongillham : the run configurations for PyCharm depend on a plugin that does the same job as your shell prefix. It means that you can develop and PyCharm automatically updates and re-runs both the api and the web application.

Documentation is sketchy and messy but hopefully better than nothing.

@docsteveharris docsteveharris linked an issue Nov 25, 2022 that may be closed by this pull request
@docsteveharris docsteveharris marked this pull request as ready for review November 29, 2022 18:30


@pytest.mark.live
def test_get_demo():
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the idea here. However I'm not so keen on unit tests that need external dependencies such as a live system to work. It potentially sets a bad precedent for future tests.

I also see the consequence of this test is that by default pyenv now needs -m 'not live' where as ideally testing live should be the exception.

I would prefer that you ripped out the live testing part and we figure out another way to do integration testing on the live or staging deployments.

@harryjmoss & @docsteveharris what are your thoughts on how strict with codebase we should be?

Copy link
Contributor Author

@docsteveharris docsteveharris Dec 1, 2022

Choose a reason for hiding this comment

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

Happy to remove this and circle back with an alternative
But what is your practice when you deploy?
We don't seem to have a testing step, and the it's not just the data that changes but because we have mock and live endpoints which have separate code then we do need a way of testing that code, and those tests can't run anywhere other than in the live environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jongillham I've removed the live pytest components so we can close this pull request 🤞. The pytest work is now on #162

force pytest installation
notes on debugging live from the GAE
distinguish tests for the live environment
filter live tests out of CI
@docsteveharris
Copy link
Contributor Author

Closing as I'm trying to tidy the branches/pull requests so that I've not mixed work from several issues into one pull request.
Apologies that @jongillham you've already started a review but this seems to be the best way
I'll try to move the comments you've made across into the new request

@docsteveharris docsteveharris deleted the steve/sql-vignette branch December 4, 2022 12:06
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.

Onboarding material for HyUI collaborators
2 participants