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

[NOT YET] All of Leo: code review #29

Closed
wants to merge 167 commits into from
Closed

Conversation

helgridly
Copy link
Contributor

@helgridly helgridly commented Sep 1, 2017

Once we've got Leo working end-to-end I'd like us to swarm on a code review and do some housekeeping tasks. This PR tracks the current state of develop against the beginning of time so it contains everything we've done.

code review date TBD but not yet

Bradley R Taylor and others added 29 commits December 8, 2017 11:50
* Add project status to README
* Add Verily to LICENSE
* Add placeholder for external contribution guidelines until we roll out our full new CLA process.
Remove a specific reference to Broad in favor of 'the copyright holder'
…ce and update Leo code to use it (#89)

* Initial refactoring. Main compiles, tests don't.

* Tests compile, don't pass

* Add DB migration, tests pass now

* Added Sam ServiceAccountProvider implementation, clean up some code

* Oops, fix name

* Beef up pet swat a little bit

* Rebase from develop

* Minor fixup

* post rebase fix

* Fix LeonardoModelCopy + ClusterKluge

* Add stub method to remove creds from instance metadata, if an override service account is used.

* Fix label swat test

* s/overrideServiceAccount/notebookServiceAccount/g

* Fix comment

* Fix substring check

* Dummy commit to test github

* PR feedback: made ServiceAccountProvider return both the Leo SA and the pem file

* Sleep a little more

* Jam a recover on removeServiceAccountKey so the delete doesn't fail

* Fix swat test flakiness, fix TODOs

* retrieving service account keys from Google is SUPER inconsistent and flakey
* Implement new endpoint in HttpSamDAO. Fix some bugs as well

* Add PetsPerProjectServiceAccountProvider

* rebase oops

* Switch unit tests to PPP

* Update libs depdendency

* Stab at a swat test case, untested

* PR feedback, and actually switch unit tests to use a PPP implementation

* Fix swat label check and init script bug

* Fix spec bug; try NOT adding dataproc.worker to the pet SA

* Fix test, temporarily

* Revert "Fix test, temporarily"

This reverts commit 937abc0.

* Put back dataproc worker

* Init script bugfix attempt. Committing to run SWAT

* More swat

* Need to set GOOGLE_APPLICATION_CREDENTIALS _conditionally_ in the docker container, not always

* Bash fail

* the env file needs to exist

* Should make swat pass

* Try and prevent spurious selenium timeouts

* Even more patience

* Increase selenium timeouts

* Update to non-SNAP
* Fix concurrency bug relating to removing Dataproc Worker IAM role from pets

* Spec

* Comment improvement

* Change method signature; add missing DB indexes

* Fix comment lie
* also removed a false comment
* Cross domain cookie auth - make the Leo proxy accept a token via cookie OR Authorization header

* Make the cookie params configurable

* Add explicit login/logout endpoints per discussion with jmandel

* Add Google client ID param to the create cluster endpoint, per discussion with jmandel

* Change notebook extension to call gapi.authorize on a timer instead of init/signIn (thanks again jmandel)

* Post rebase fixes

* Change cookie logic to be more correct

* Minor fixes

* Remove googleClientId from the API and templating logic, in its own commit

* Stab at postMessage implementation to pass the client ID to the notebook extension. Swat still in progress.

* Temporarily comment out allowedOrigins to facilitate testing

* Revert "Temporarily comment out allowedOrigins to facilitate testing"

This reverts commit 73c1177.

* PR feedback, and add swagger

* Need to set CORS headers for cross-domain cookies

* Fix unit test

* Support preflight OPTIONS requests for ajax

* ProxyRoutes bug

* Hopefully fix my proxy routes woes

* slight CORS fix, getting close

* Missed a couple cors headers

* Make postMessage data an object to be consistent

* Dummy page for testing. It works against a fiab :D. Needs swatification

* Oops, committed an .orig file

* JS tweaks and do cors for all proxy requests

* Fix tests

* dummy client fixes

* SWAT

* Debug error on Jenkins

* implement awaitLoaded in DummyClientPage

* Post rebase fixes

* Minor cleanup

* docker fail

* Check postMessage origin in dummy client
* give clusterserviceaccount access to dataproc bucket
* Change cookie name: FCtoken -> LeoToken

* Just check the origin
* Add Bigquery scopes to Dataproc cluster SA
* BigQuery Swat test
* Update formatting for new version of executeCell()
* Copy Google Role endpoints from UI-Repo Swat
* Bump handleResponse/awaitResponse times to 1 sec and billing account creation to 10 min
* Dockerfile reduction and refactoring

* docker seventeen

* Split Dockerfile into separate layers for prereqs + Java + Spark/Hadoop/Hail + pips

* Dockerfile updates based on feedback

* I think I need to mount Hive

* More spark confs that seem to help

* More tries

* useless Java option

* Reduce spark.yarn.am.memory

* Link JIRA in comment
* Add println

* Increase route timeout
* initial - probs doesn't work

* make the notebook action strings correct

* fix execution context bug

* try a different way of authing

* i dunno

* logging stuff

* fiab stuff

* trying to figure out why resourceAction is giving me a false

* hmmm - are we actually getting a boolean and passing in the right stuff?

* remove whitelist D:

* approximately

* import statements

* stuff

* add back recreate cluster

* fixing samauth test

* add cache stuff to config

* make it actually talk to Sam

* oops

* use a real fake pem

* ignore test for now...

* forgot this

* update for sam changes

* resolve conflicts

* let's not be too clever

* tiny correction

* log some stuff

* will this work idk

* decode

* logging stuff

* more logging

* add back whitelist + update for sam changes

* do it right

* log more things

* maybe, idk

* omg this might work i think

* no, but maybe this?

* wild guesswork

* this has to be it

* as json this time

* try this

* idk

* oh this actually worked

* comments and test stuff

* switch auth for tests

* src

* im sleepy

* some initial review changes

* Change strings to model classes

* change tests

* oops broke tests

* new auth provider interface

* fix whitelist

* fix reference.conf

* fix sam server url

* use mockLeoAuthProvider in LeonardoServiceSpec for now

* go back to whitelist

* sam auth provider spec

* list clusters courtesy of Rob and Hussein

* override canSeeAllClustersInProject

* hussein's nitpick + executionContext

* bracket

* samclient prelim

* move project and notebook actions back to SamAuthProvider

* [nomerge] Better LeoService interop w/ auth providers (#120)

* new auth provider interface

* wip

* better

* commentary

* tests for optimized leo list clusters

* whitelist fixes

* mockito test

* removing extra stuff from commontestdata

* silly

* fix tests

* fix tests for real

* maybe don't use mockito

* add billing project manually to mock

* tiny mistake

* maybe this

* I can't seem to do anything right

* now we're getting somewhere

* wait no, add back mockito

* why can't this be over

* what's in a name

* stop reading my commit messages, the pressure to entertain is too much

* I miss the days when my commit messages were just for me

* please pass

* clean it up make it pretty

* get and contain issue

* that which we call a rose
By any other word would smell as sweet but would probs be confusing

* change artifactory

* review suggestions

* why would I get it right the first time

* something broke again

* fix for notebookclusters too

* stupid race conditions

* change cacheExpiryTime to FiniteDuration

* fix reference.conf cacheExpiryTime

* ergh

* missed one

* maybe this

* I think I'm getting closer

* well, maybe

* update sam swagger client version

* so muh simpler

* oops naming resources

* remove logging

* fix the automated tests

* have more patience

* typo

* remove logging messages

* do the simple thing first

* fix the owner delete bug

* don't need userEmail in internalDeleteCluster anymore

* pass in full cluster to notify methods

* fix tests

* go back to using email project and cluster name

* refactor swat tests a bit

* missed two...

* change it again sigh

* fix tests
* Make notebooky tests reuse the same cluster
- new LeonardoTestUtils trait
- Run ClusterMonitorSpec in parallel
- Don't need to explicitly mixin WebBrowserSpec
- centralize hail file vals
- Ensure withOpenNotebook/withNewNotebook always terminate their kernels
- Removed redudant test which uploads but doesn't execute notebook
* use workbench libs

* auth token for leo, other deletions, tweaks

* adding methods to configs

* adding some more info

* forgot to render users.json

* build/push swat tests docker image in jenkins build script

* all up to date

* syncing users json

* Add campaignManagers to users

* workbench-libs needs these (TODO remove)

* fix for app conf

* real fix for app conf

* Hermione and Ron

* more app conf fix

* more app conf ctmpl

* Update scalatest version for workbench-libs compatibility

* implicits are the devil

* dammit weasley

* MORE PATIENCE

* fix build script when pushing to develop

* save git SHA during build script
Added link to the production swagger UI to README.md
* Add error handling to providers

* Add tests

* Clean up

* Add logging

* Exception recovery not needed on SamAuthProvider

* Better specs and error handling

* Add comment

* Log text nit

* Add comment to specs
* Delete cluster resources in Sam only after deletion is complete in Google and the Leo DB
* Integration Tests: Downgrade selenium/node-chrome to 3.8.1-erbium
@rtitle rtitle closed this Feb 1, 2018
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

9 participants