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

Common code between the different python backends #5164

Conversation

kimwnasptd
Copy link
Member

Right now JWA's backend is written in Python Flask. We also have the Tensorboards web app whose backend will also be written in Python and Flask. Lastly, the Volumes web app, that we'd like to contribute, will be also written with the same technologies.

I think it would be a good next step to create a common python codebase/library that all these apps could use, at some extend. This common code will include:

  • How we handle exceptions in the backend
  • Common routes for serving static files and their cache control policy
  • Authorization checks with SubjectAccessReview
  • Authentication checks on the Kubeflow headers
  • Common helper functions for dates, yaml parsing etc
  • health/liveness probes

I have prepared this code while working on the Volumes web app and it's the first step towards pushing the backend code upstream. It would also help our running GSoC project with Tensorboard as the backend could use a lot of this common code. The Jupyter web app should also gradually use this code.

I'm also adding @kandrio98 to the loop since this affects his GSoC project as well as to get him more exposed to our review process.

@jlewi if you have a better candidate in mind for reviewing this PR let me know

/cc @jlewi
/assign @kimwnasptd

@kubeflow-bot
Copy link
Contributor

This change is Reviewable

@kimwnasptd kimwnasptd force-pushed the feature-kimwnasptd-wa-common-backend-code branch from aa243fa to 8325778 Compare July 27, 2020 12:14
@jlewi
Copy link
Contributor

jlewi commented Jul 27, 2020

@kimwnasptd Who else will be owning and maintaining this code? They should be the ones reviewing it.

What about @StefanoFioravanzo and what about @lalithvaka

@StefanoFioravanzo
Copy link
Member

@jlewi I think you are suggesting creating an OWNERS file for the py/ directory? It would definitely make sense since we have are going to rely heavily on this code for various applications. It's ok for me to be part of that file, @elikatsis as well is involved in the development of this common code. We could start with us + @kimwnasptd being part of the OWNERS

@jlewi
Copy link
Contributor

jlewi commented Jul 29, 2020

What project or application is this code for? Please open up a separate PR creating an appropriate directory with an OWNERs file. The OWNERs should indicate who will be responsible for reviewing and maintaining this code.

If this code is primarily for the jupyter web app, why not put it in
https://github.com/kubeflow/kubeflow/blob/master/components/jupyter-web-app/OWNERS

@kimwnasptd
Copy link
Member Author

@jlewi this will be common backend code between the jupyter web app, tensorboards web app and the volumes web app.

The jupyter web app will start using the common code more gradually over time, but the tensorboards and volumes web apps will be using this common code out of the box.

@jlewi
Copy link
Contributor

jlewi commented Jul 31, 2020

Can we structure the code so that all code lives under an appropriate project directory with a suitable OWNERs file that indicates who is responsible for this project.

All projects will need to find WG sponsorship or they will be archived. Ownership of the projects and boundaries should be clearly delineated by directory structure and OWNERs files.

@kimwnasptd
Copy link
Member Author

@jlewi since this will be common code for different projects, what structure do you think would fit best here?

For the approvers/reviewers we could add @StefanoFioravanzo @elikatsis and I. Once we decide on where this code should be placed I'll create a new small PR just for adding the OWNERS file in the agreed place[s].

@jlewi
Copy link
Contributor

jlewi commented Aug 3, 2020

@kimwnasptd I think my comment above applies.

Can we structure the code so that all code lives under an appropriate project directory with a suitable OWNERs file that indicates who is responsible for this project.

So pick an appropriate directory to serve as the parent directory for all the web apps that you will be developing. Put an OWNERs file there to indicate clear ownership and then organize the code under that.

@kimwnasptd
Copy link
Member Author

I updated the code to live under the crud-web-apps directory.

@elikatsis @StefanoFioravanzo take a look when when you have the time. I'll be on PTO for the next couple weeks so I won't be able to respond frequently during that time, but I'll try to answer any questions that might come up.

/cc @elikatsis
/cc @StefanoFioravanzo
/uncc @jlewi

@k8s-ci-robot k8s-ci-robot requested review from elikatsis and StefanoFioravanzo and removed request for jlewi August 6, 2020 17:16
Create a python module under the kubeflow.kubeflow package that will
be exposing common code and a base app the takes care of:

* Exceptions handling
* Common routes for serving static files and their cache control policy
* Authorization checks with SubjectAccessReview
* Authentication checks on the Kubeflow headers
* Common helper functions for dates, yaml parsing etc
* health/liveness probes

Backends that are written with Python/Flask should use this common code
in order for us to reduce code duplication and have our backends align
with our accepted practices.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
@kimwnasptd kimwnasptd force-pushed the feature-kimwnasptd-wa-common-backend-code branch from bbf24ef to 24f23d7 Compare August 6, 2020 17:20
@StefanoFioravanzo
Copy link
Member

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: StefanoFioravanzo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit e99b5e1 into kubeflow:master Aug 7, 2020
2 of 3 checks passed
@elikatsis elikatsis deleted the feature-kimwnasptd-wa-common-backend-code branch August 28, 2020 12:12
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
Create a python module under the kubeflow.kubeflow package that will
be exposing common code and a base app the takes care of:

* Exceptions handling
* Common routes for serving static files and their cache control policy
* Authorization checks with SubjectAccessReview
* Authentication checks on the Kubeflow headers
* Common helper functions for dates, yaml parsing etc
* health/liveness probes

Backends that are written with Python/Flask should use this common code
in order for us to reduce code duplication and have our backends align
with our accepted practices.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 12, 2021
Create a python module under the kubeflow.kubeflow package that will
be exposing common code and a base app the takes care of:

* Exceptions handling
* Common routes for serving static files and their cache control policy
* Authorization checks with SubjectAccessReview
* Authentication checks on the Kubeflow headers
* Common helper functions for dates, yaml parsing etc
* health/liveness probes

Backends that are written with Python/Flask should use this common code
in order for us to reduce code duplication and have our backends align
with our accepted practices.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants