Skip to content

Conversation

kofalt
Copy link
Contributor

@kofalt kofalt commented Sep 21, 2017

A big problem with exposing our functionality is that everything has a 5x multiplier: AddAcquisitionNote. This blocks doing some useful stuff in the SDK because the functions would be irritating and confusing.

We should provide /container/x routes that figure out which type of container we mean, and just serve the equivalent route. This PR is an incomplete version of doing just that!

Solved problem: we need an efficient way to resolve the container noun. Might be solved for free if we ever can truly homogenize containers. While the func snippet + db command approach here is a bit ugly on the code, I can confirm it is quite efficient and safe to use.

Unsolved problem: make webapp2 just serve a different handler, dangit. The commented-out self.redirect approach totally works: you can GET api/container/x/sessions where x is a project ID. But that won't work for non-gets, and we shouldn't redirect anyway. We should just serve.

I'm 90% sure we can take these code scraps, and either:

  1. Modify the (~5) fields of the request object to reflect the "proxying"
  2. Construct a new context with the same body reader?

And call a handler method. Help?

@scitran scitran deleted a comment from codecov-io Sep 21, 2017
@kofalt kofalt added the SDK label Oct 24, 2017
@ambrussimon ambrussimon force-pushed the super-awesome-abstract-container-route branch 2 times, most recently from 543a0a7 to 4cabccf Compare January 23, 2018 08:49
@codecov-io
Copy link

codecov-io commented Jan 23, 2018

Codecov Report

Merging #937 into master will increase coverage by 0.01%.
The diff coverage is 97.61%.

@@            Coverage Diff             @@
##           master     #937      +/-   ##
==========================================
+ Coverage    90.8%   90.82%   +0.01%     
==========================================
  Files          50       51       +1     
  Lines        7029     7054      +25     
==========================================
+ Hits         6383     6407      +24     
- Misses        646      647       +1

"groups": db.getCollection("groups").findOne({"_id" : _id}, {"_id": 1}),
"projects": db.getCollection("projects").findOne({"_id" : _id}, {"_id": 1}),
"sessions": db.getCollection("sessions").findOne({"_id" : _id}, {"_id": 1}),
"acquisitions": db.getCollection("acquisitions").findOne({"_id" : _id}, {"_id": 1})
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll probably want to add analyses to this list as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Collections, Y/N ?

@ambrussimon ambrussimon force-pushed the super-awesome-abstract-container-route branch from 4cabccf to 0610248 Compare January 26, 2018 16:19
@kofalt
Copy link
Contributor Author

kofalt commented Jan 26, 2018

So far, I tried swapping out SDK tests for acquisitions, and it works 🎉

More to follow.

@kofalt
Copy link
Contributor Author

kofalt commented Aug 7, 2018

This was later completed in flywheel-io/core#937

@kofalt kofalt closed this Aug 7, 2018
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.

6 participants