This repository has been archived by the owner on Nov 7, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dcwalk
reviewed
Jul 24, 2017
id, url,created,updated,creator_key_id, | ||
name,email,event_name,agency_name, | ||
agency_id,subagency_id,org_id,suborg_id,subprimer_id, | ||
ftp,database,interactive,many_files, | ||
comments | ||
from uncrawlables | ||
where url = $1;` | ||
FROM uncrawlables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why uncrawlables?
dcwalk
reviewed
Jul 24, 2017
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
Once again, in no way comprehensive or technical --
- wondering at the choice of uncrawlables for terminology?
- I don't always understand why code bits are commented out (will they go away eventually? be reintroduced?)
- should some TODOs inline make their way to issues?
Thx for review!
|
dcwalk
approved these changes
Aug 3, 2017
This was referenced Aug 3, 2017
Thx team, merging! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR allows collections to scale, and makes collections do cooler stuff. It relies on datatogether/sql_datastore#2 to do that cooler stuff.
In the interest of getting the Collections feature out the door, I hacked in a very-terrible implementation of collections where they were stored as JSON on the collections table. This is bad. Won't scale. Will be slow. Badbadnotgood.
So here we refactor away the bad bits, replacing them with paginated collection items, allowing collections to scale up to 10s / 100s of thousands of entries if needed.
Cool. "Scale". Whatever. The more interesting part is that collection items are now composed of entries of Urls, which we can do more interesting things with. For example, if I add a url to my collection, and you add the same url to your collection, they're now formally linked. If Sentry crawls that link and updates the status of that URL, it updates for both of our collections. I can make notes about that url in my collection, and you can do the same in yours, and those notes won't get crossed, but we can create views that show both of our notes on that urls. If I add metadata about that Url, your collection can see it. If that url is in a primer, we can surface that too.
Things are (hopefully) about to get fuego once we surface this stuff in the frontend.