Meeting Minutes (November 3, 2013)

Announcements:

Consider how far you are in the project, how much you feel you have left to do. Consider how much time you need to spend in other courses, and how much time we’ll need to review your code. Factor all this in and course-correct if necessary. And if things are looking sour, let us know ASAP.

If you are blocked on reviews, come (gently) bug us.

Elaine M:

Is it better to have captured variables in the url, or pass them as parameters?

It depends on what you’re doing. For RESTful stuff, you pass the parameters for the things you want in the URL for a GET, where you are trying to retrieve information.

Think of a URL as a fixed location. Any identifying information that represents that location should be in the URL. Optional values, for, say, filtering lists, would be query parameters.

So if I create a new checklist for request 103, it should be something like: checklist/new/103, rather than checklist/new/request=103?

No, you never decide on ID’s and URLs shouldn’t imply actions. That’s what HTTP methods are for. So to create something, you do a POST to a list URL. So POST checklist/ with a payload for whatever data you want to use for the creation. The resulting payload from the server will represent that checklist, and as part of that, you will get a link to where it lives (like checklists/123/)

Is it only with GET where I actually have to use parameters?

A good resource to look at would be webapi/resources/default_reviewer.py. Don’t think of them as parameters. Think of them as addresses, locations.

Note that our WebAPI resource code makes doing this pretty simple – it takes care of a lot of the boilerplate for you. Checklists/123/ is a fixed address for that specific instance of the resource. It could even be checklists/my-name/, if the WebAPIResource class said that was the address. Though database-backed IDs are usually best.

WebAPI stuff can be pretty tricky, but we’ve also done quite a bit of work with it recently, so a lot of webapi stuff is swapped into our brains. Ask us questions when you’re stuck. Just follow the pattern that default_reviewer.py provides for get_queryset, create, and update, and it’ll do most of the rest.

Natasha:

I just posted my first front-end review request. At one place I’m just repeating stuff that I had put into a function because I couldn’t figure out how to call the function form the right scope.

We will do a review pass today. If you’re way off rails, I’ll let you know.

I haven’t gotten my Mac dev environment set up.

We will sort out your dev environment stuff right after the meeting. Basically, we’re probably gonna point you at the vagrant stuff which is pretty straight-forward.

Edward:

I noticed r/4662 is marked as submitted, but there are still some outstanding issues, so I’m a bit confused.

It seems like someone closed it by accident, feel free to re-open it.

I’d like to add an option to rbt post so that it automatically opens the URL after a review request is posted. Just want to get some feedback on this idea. I’m still looking into other project ideas but I think this is a quick and useful one.

It already exists: -o

If you’re looking for additional projects, I think prototyping out a new search implementation would be pretty sweet. It would make a major difference to people.

It’s probably too big to rewrite search, but a prototype that built up some knowledge of how Django-haystack works would be awesome.

It’s mostly backend – it’d be indexing. I imagine the search UI would be more or less the same. Possibly some different things in the auto-suggest.

Maybe start checking out haystack and get familiar with it. And start coming up with ideas on how we could prototype its usage in RB.

Sounds like a good idea. What files should I start looking into for the current search implementation?

reviewboard/reviews/management/commands/index.py is the existing lucene-based indexing code and ReviewsSearchView in reviewboard/reviews/views.py is what does the front-end search. There’s no dynamic stuff for search right now, it’s just a GET with a query string that returns a page of results.

Haystack is not currently used in RB. The existing search was written before haystack existed and requires that people compile pylucene, which is a huge mess of java. I think we’d like a new implementation to use haystack with the whoosh backend (at least by default), so that it’s all pure python. Most people don’t set up search because of all that and when they do, it often doesn’t work, because (py)lucene is bad at backwards compatibility.

Behzad:

I got most of my questions answered yesterday. I have a few more though: do we want to allow for a review request to be able to have more than one trophy?

I think it’s going to be inevitable as we create more trophy types that there’ll be an overlap.

I’m not sure how to put of new iterations of my code in the same review request.

rbt post takes an -r argument for which you can pass an ID. That’ll upload your current branch vs. master diff as a review request draft. Look it over, make sure you dig (and re-post if you need to make fixes).

Alissa:

Would it be ok to do a blog post about web vulnerabilities?

That would be great.

Are there any examples in the application of files being uploaded/downloaded?

Django has a concept of a FileStorage, which FileField uses for uploading. You can write to a path using it, kind of like standard file I/O. You should be able to write to $MEDIA_ROOT/uploaded/files/, maybe in a _security directory or something.

For downloading, there are two ways to do this:

1. The Python code can open a request to the server itself is on and fetch the files (allowing all the logic to remain in the Python part of this).

2. It can emit JavaScript that makes the attempt (success/fail reporting has to be a little more controlled on the JavaScript side).

The first option is probably simpler and nicer. It does mean that certain browser quirks (like IE’s attempts to ignore mimetypes and sniff contents) won’t be checked. Even then it would only be checked if the admin is running IE, which seems less likely. So #1 is fine.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s