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

Add origin tracking to files #193

Merged
merged 3 commits into from
Mar 15, 2016
Merged

Add origin tracking to files #193

merged 3 commits into from
Mar 15, 2016

Conversation

kofalt
Copy link
Contributor

@kofalt kofalt commented Mar 11, 2016

All files now have an origin map, with a type string (device, user, job, or unknown) and an id field pointing to the corresponding document. If a file was stored before this change, the type returns unknown.

In addition, routes that return an object, or objects, with a files array, can now take an optional join=origin request parameter. This will add a join-origin key to each object, containing a context map of the various origin documents. Use this to prevent N calls to various other endpoints.

Example:

// on the file object
 "origin": {
      "type": "user",  // can be user | device | job | unknown
      "id": "[email protected]"
    }

Example of a join-origin:

{
    "files": [ 
        { "origin": { "type":"user", "id":"foo" }}
        // ... 
    ],

    "join-origin": {
        "users":    {"foo": obj, "bar": obj },
        "devices":  {"foo": obj},
        "jobs":     {"foo": obj}
    }
}

Along the way, the job generator had to gain additional insight to throw job IDs onto upload URLs, the Placer gained an origin field for placing files, not-Placer uploads had origin hacked into them, and the base request init now knows a bit more about the requester.

Closes #163.

@kofalt kofalt force-pushed the origin branch 3 times, most recently from d5a502b to a46dad2 Compare March 14, 2016 18:25
@nagem
Copy link
Contributor

nagem commented Mar 14, 2016

LGTM - I like the enum for origin type.

@@ -394,4 +394,4 @@ def packfile(self, cont_name, **kwargs):
else:
raise Exception('Not authorized')

return upload.process_upload(self.request, upload.Strategy.packfile)
return upload.process_upload(self.request, upload.Strategy.packfile, None, None, self.origin)
Copy link
Contributor

Choose a reason for hiding this comment

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

to avoid this None, None, you can do origin=self.origin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great point. I don't like doing stuff like this because the params inevitably change and I mess up :) Changing now.

@rentzso
Copy link
Contributor

rentzso commented Mar 15, 2016

LGTM - just a couple of comments.

@kofalt
Copy link
Contributor Author

kofalt commented Mar 15, 2016

Comments addressed and rebased.
I was going to call it good for merge, but when testing a packfile upload:

  File "webapp2.py", line 570, in dispatch
    return method(*args, **kwargs)
  File "./api/handlers/listhandler.py", line 399, in packfile
    return upload.process_upload(self.request, upload.Strategy.packfile, origin=self.origin)
  File "./api/upload.py", line 80, in process_upload
    placer.check()
  File "./api/placer.py", line 163, in check
    validators.validate_data(self.metadata, 'packfile.json', 'POST')
  File "./api/validators.py", line 29, in validate_data
    validator = from_schema_path(schema_url)
  File "./api/validators.py", line 101, in from_schema_path
    schema, resolver = _resolve_schema(schema_url)
  File "./api/validators.py", line 67, in _resolve_schema
    base_uri, schema_name = re.match('(.*/)(.*)', schema_url).groups()
AttributeError: 'NoneType' object has no attribute 'groups'

Any thoughts on this @rentzso?
I only ran into it after rebasing, so it's a good thing I did 😄

I'll be out for a decent chunk of today, so if you have ideas feel free to throw them on this branch.
Otherwise we can work on it later.

@rentzso
Copy link
Contributor

rentzso commented Mar 15, 2016

@kofalt I will check. It is related to the new validation endpoint for serving schemas.

@kofalt
Copy link
Contributor Author

kofalt commented Mar 15, 2016

New commit 6b5a42a has fixed the problem as far as I can tell. Merging.

kofalt added a commit that referenced this pull request Mar 15, 2016
Add origin tracking to files
@kofalt kofalt merged commit 4e112e4 into master Mar 15, 2016
@kofalt kofalt deleted the origin branch March 15, 2016 18:13
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.

3 participants