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

Add new page about federation #829

Merged
merged 3 commits into from Jul 21, 2022
Merged

Add new page about federation #829

merged 3 commits into from Jul 21, 2022

Conversation

sebbacon
Copy link
Contributor

Also sync precommit version with .python-version, so I can commit it

@render
Copy link

render bot commented Jun 28, 2022

@render
Copy link

render bot commented Jun 28, 2022

A deploy for your Render PR Server at https://opensafely-documentation-pr-829.onrender.com just failed.

View details on your dashboard at https://dashboard.render.com/static/srv-cath0lvh8vlfhb4appjg.

@render
Copy link

render bot commented Jun 28, 2022

Your Render PR Server at https://opensafely-documentation-pr-829.onrender.com is now live!

View it on your dashboard at https://dashboard.render.com/static/srv-cath0lvh8vlfhb4appjg.

@sebbacon sebbacon force-pushed the federation-info branch 2 times, most recently from 6fef021 to 74ed3c5 Compare June 28, 2022 15:05
@render
Copy link

render bot commented Jun 28, 2022

Your Render PR Server at https://opensafely-documentation-pr-829.onrender.com is now live!

View it on your dashboard at https://dashboard.render.com/static/srv-cath0lvh8vlfhb4appjg.

1 similar comment
@render
Copy link

render bot commented Jun 29, 2022

Your Render PR Server at https://opensafely-documentation-pr-829.onrender.com is now live!

View it on your dashboard at https://dashboard.render.com/static/srv-cath0lvh8vlfhb4appjg.

@render
Copy link

render bot commented Jun 29, 2022

Your Render PR Server at https://opensafely-documentation-pr-829.onrender.com is now live!

View it on your dashboard at https://dashboard.render.com/static/srv-cath0lvh8vlfhb4appjg.

1 similar comment
@render
Copy link

render bot commented Jul 12, 2022

Your Render PR Server at https://opensafely-documentation-pr-829.onrender.com is now live!

View it on your dashboard at https://dashboard.render.com/static/srv-cath0lvh8vlfhb4appjg.

Copy link
Contributor

@evansd evansd left a comment

Choose a reason for hiding this comment

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

Looks great to me, for what that's worth!


One problem OpenSAFELY solves is that backends may present very different environments and databases: for example, TPP provides their data in a _Microsoft SQL Server_ database on a hardware server, and EMIS provides it via their _EXA_ analytic platform in the cloud. OpenSAFELY tools hide these differences from researchers, so that their code can be run **everywhere**. After code is run on a backend, its outcomes are carefully checked for safety within the same secure perimeter, and finally released so that they can be **recombined**:

```mermaid
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: It's nice that you've used the built-in diagram support 👍

There might be others elsewhere in the docs that we can replace.

This does make a request to unpkg.com to retrieve the JavaScript which gives me two thoughts:

  • Might this be blocked for users if they are on a work IT network?
  • Are there privacy issues we care about? (Perhaps not: we already use Google Fonts.)

It is possible to host the JavaScript ourselves although we then need to manually keep it in sync.

I'm undecided, but worth a thought.

Copy link
Contributor

Choose a reason for hiding this comment

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

Blocking the request to unpkg.com means you just see the raw Mermaid source:

mermaid-unpkg-blocked-request

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've noticed an unpleasant FOUC type effect with mermaid; I wonder if hosting the javascript ourselves would solve this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about the privacy plugin as a solution? I'm quite keen to pay for mkdocs given how much use we've made of it, anyway

@@ -1,5 +1,5 @@
default_language_version:
python: python3.10
python: python3.8
Copy link
Contributor

Choose a reason for hiding this comment

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

comment: I don't think this should change.

It's applied to the databuilder/ directory and I think this does want to use newer Python.

In the worst case, you can commit via git commit --no-verify, but maybe we need to look into this again. It's the mess of dealing with multiple Python versions in the same repository, unfortunately.

I did have a look at solving this in #832, but didn't get round to tidying that up yet. It's once again another issue that it would solve.

@StevenMaude
Copy link
Contributor

I've added a couple of small comments for polish ✨ and I don't think the .pre-commit-config.yaml should change here.

But no issues with the actual written content which looks good 👍

@StevenMaude StevenMaude changed the title New page about federation Add new page about federation Jul 12, 2022
@render
Copy link

render bot commented Jul 19, 2022

Your Render PR Server at https://opensafely-documentation-pr-829.onrender.com is now live!

View it on your dashboard at https://dashboard.render.com/static/srv-cath0lvh8vlfhb4appjg.

Also sync precommit version with .python-version, so I can commit it
@render
Copy link

render bot commented Jul 21, 2022

Your Render PR Server at https://opensafely-documentation-pr-829.onrender.com is now live!

View it on your dashboard at https://dashboard.render.com/static/srv-cath0lvh8vlfhb4appjg.

@sebbacon sebbacon merged commit 1137074 into main Jul 21, 2022
@sebbacon sebbacon deleted the federation-info branch July 21, 2022 15:04
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.

None yet

3 participants