Project

Profile

Help

Story #86

closed

As a user, I can add mutable custom metadata to a unit

Added by mhrivnak about 9 years ago. Updated over 2 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
High
Category:
-
Sprint/Milestone:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Platform Release:
2.7.0
Groomed:
No
Sprint Candidate:
No
Tags:
Pulp 2
Sprint:
January 2015
Quarter:

Description

When using pulp as a data source for another application, like a website or even a web UI, it would be very useful for the user to have a blob of metadata on each unit that they can manipulate. The "user" in this case might be the web app itself.

Deliverables

  • add a blob to the unit data model
  • make that blob readable and mutable through the REST API
  • REST API docs
  • release notes
Actions #1

Updated by cduryee about 9 years ago

Would this blob ever be synced down from a feed url or published?

Actions #2

Updated by bcourt about 9 years ago

Is this blob on the unit, the repo_unit, or both. I.E. does the data apply to the individual unit, the repositories representation of that unit, or both?

Actions #3

Updated by mhrivnak about 9 years ago

The blob would not be sync'd or published.

I think that starting with the blob on the unit is simpler, especially because we don't have to do anything with regards to promotion. If someone really wants it on the unit association also, we can make that a second pass.

Actions #5

Updated by cduryee about 9 years ago

  • Sprint/Milestone set to 9
Actions #6

Updated by rbarlow about 9 years ago

  • Status changed from NEW to ASSIGNED
Actions #7

Updated by rbarlow about 9 years ago

  • Assignee set to rbarlow
Actions #8

Updated by rbarlow about 9 years ago

I'm planning to name this blob "user_metadata" and make it a blank dictionary by default, unless anyone has a better suggestion.

Actions #9

Updated by cduryee about 9 years ago

will there be enforcement of how much data can be stored in there?

Actions #10

Updated by rbarlow about 9 years ago

On 01/12/2015 06:19 PM, Chris Duryee wrote:

will there be enforcement of how much data can be stored in there?

I am not planning on enforcing anything about the request outside of
requiring the usual UPDATE permission bit. Mongo itself will reject the
data if it goes over its limit (16 MB I believe). I'm planning to make
it so this API can only do HTTP 200, 403, and 404 with a PUT request to
something like:

/content/units/<UUID>/user_metadata/

Any user that's allowed to UDPATE that URL can overwrite the
user_metadata field for an already existing UUID. No validation of any
kind will be provided.

Does that sound reasonable?

Actions #11

Updated by cduryee about 9 years ago

Yeah, I think that is reasonable. It may be good to note in the doc that the data lives in mongo and not on the local filesystem. Hopefully this will discourage people from uploading uuencoded binaries (other than maybe an icon) into the field:)

Actions #12

Updated by rbarlow about 9 years ago

On 01/12/2015 08:14 PM, Chris Duryee wrote:

It may be good to note in the doc that
the data lives in mongo and not on the local filesystem. Hopefully this
will discourage people from uploading uuencoded binaries (other than
maybe an icon) into the field:)

Will do!

Actions #13

Updated by rbarlow about 9 years ago

  • % Done changed from 0 to 70
Actions #14

Updated by rbarlow about 9 years ago

I have completed all but two of the objectives. One of them is tests, I will do that tomorrow.

The other is "add a blob to the unit data model". This one is a little problematic. The unit data model doesn't exactly exist, at least not in terms of what the word "data model" typically means. There are some models for plugins to use to represent units to the plugins, but these models do not match the representation in the database. Instead they are a transformation of that data. Without a solid data layer, this is tricky to achieve and may result in issues.

I've opted to do this instead:

  • Add a REST endpoint at /pulp/api/v2/content/units/<type_id>/<unit_id>/user_metadata/ that accepts GET and PUT requests.
  • The GET view will 404 if the unit does not exist, or if the model does not have a user_metadata field (i.e., no user has ever PUT one).
  • The PUT will set the unit's user_metadata field to the given parameters. The PUT will 404 if the unit does not exist.
  • Once a user_metadata exists on a unit, it cannot be unset as there is no DELETE. However, users can set it to {} if they like.

Is this all acceptable? I wouldn't choose this path if we had a data layer model for units, but this avoids a lot of changes that would be necessary if we wanted to accomplish all units always having his field. I believe those changes are risky and time consuming compared to doing it this way.

It wouldn't be too hard to add a DELETE if we wanted it, though I couldn't think of an obvious use case to do it and less testing and documentation is necessary if I don't.

Actions #15

Updated by rbarlow about 9 years ago

mhrivnak and I had an epic conversation this morning about this story. Here are some conclusions we came to:

1) I'll name the database field _pulp_user_metadata instead of user_metadata so that it has less chance of collision.
2) I'll modify Unit to try to ensure that this field is present on all Units.
3) It will be part of Unit.metadata, unless this later causes issues.
4) I'll write a migration to add this to all existing units.
5) The API endpoint will still be just user_metadata.

Actions #16

Updated by rbarlow about 9 years ago

  • % Done changed from 70 to 40
Actions #17

Updated by rbarlow about 9 years ago

We had one more discussion, where we decided to call it pulp_user_metadata internally and at the endpoint so that everything is consistent.

Actions #18

Updated by rbarlow about 9 years ago

  • % Done changed from 40 to 60
Actions #19

Updated by rbarlow about 9 years ago

  • Status changed from ASSIGNED to POST
  • % Done changed from 60 to 90
Actions #20

Updated by rbarlow about 9 years ago

  • Platform Release set to master

Added by rbarlow about 9 years ago

Revision 8564bb93 | View on GitHub

Add an API for storing custom Unit metadata.

https://pulp.plan.io/issues/86

closes #86

Added by rbarlow about 9 years ago

Revision 8564bb93 | View on GitHub

Add an API for storing custom Unit metadata.

https://pulp.plan.io/issues/86

closes #86

Actions #21

Updated by rbarlow about 9 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 90 to 100

Applied in changeset pulp|commit:8564bb935e029203ae739f61166b1d4eed88dc07.

Added by rbarlow about 9 years ago

Revision a7cc9404 | View on GitHub

Fix broken unit tests due to a change in platform.

re #86

Actions #22

Updated by rbarlow about 9 years ago

I also needed to make this change in pulp_rpm to respond to these changes:

https://github.com/pulp/pulp_rpm/pull/643

Actions #23

Updated by rbarlow almost 9 years ago

  • Platform Release changed from master to 2.7.0
Actions #24

Updated by dkliban@redhat.com almost 9 years ago

  • Status changed from MODIFIED to 5
Actions #26

Updated by ipanova@redhat.com over 8 years ago

  • Groomed set to No
  • Sprint Candidate set to No
Actions #27

Updated by ipanova@redhat.com over 8 years ago

  • Status changed from 5 to ASSIGNED

Tested:
1) retrieval of pulp_user_metadata of existent unit_id(200)
2) retrieval of pulp_user_metadata of non-existent unit_id(404)
3) set/update pulp_user_metadata of existent unit_id(200)
4) set/update pulp_user_metadata of non-existent unit_id(404)
5) confirm that post, delete don't work(405)
6) confirm that pulp_user_metadata is present in content types, and it is {} by default --- FAILED
Rpm content unit type has by default this field and it is empty dict, meanwhile puppet_module does not have this field until the put request was made.

rpm -qa pulp-server
pulp-server-2.7.0-0.3.beta

# curl -H "Accept: application/json" -H "WebFrameworkSwitch: django" -X GET -k -u admin:admin 'https://localhost/pulp/api/v2/content/units/puppet_module/9011a43d-2cde-4b1f-a780-970c7eb6fca4/'|python -m json.tool
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   706  100   706    0     0   2006      0 --:--:-- --:--:-- --:--:--  2011
{
    "_content_type_id": "puppet_module",
    "_id": "9011a43d-2cde-4b1f-a780-970c7eb6fca4",
    "_last_updated": "2015-07-16T10:21:43Z",
    "_storage_path": "/var/lib/pulp/content/puppet_module/yelp-netstdlib-0.0.1.tar.gz",
    "author": "yelp",
    "checksum": null,
    "checksum_type": "sha256",
    "checksums": [],
    "children": {},
    "dependencies": [
        {
            "name": "puppetlabs/stdlib"
        }
    ],
    "description": "A collection of network functions and facts",
    "license": "Apache License, Version 2.0",
    "name": "netstdlib",
    "project_page": "https://github.com/Yelp/puppet-netstdlib",
    "source": "https://github.com/Yelp/puppet-netstdlib.git",
    "summary": "A collection of network functions and facts",
    "tag_list": null,
    "types": [],
    "version": "0.0.1"
}
[root@ec2-54-78-44-32 ~]# curl -H "Accept: application/json" -H "WebFrameworkSwitch: django" -X GET -k -u admin:admin 'https://localhost/pulp/api/v2/content/units/puppet_module/9011a43d-2cde-4b1f-a780-970c7eb6fca4/pulp_user_metadata/'|python -m json.tool
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1272  100  1272    0     0   3258      0 --:--:-- --:--:-- --:--:--  3261
{
    "_href": "/pulp/api/v2/content/units/puppet_module/9011a43d-2cde-4b1f-a780-970c7eb6fca4/pulp_user_metadata/",
    "error_message": "'pulp_user_metadata'",
    "exception": [
        "KeyError: 'pulp_user_metadata'\n"
    ],
    "http_request_method": "GET",
    "http_status": 500,
    "traceback": [
        "  File \"/usr/lib/python2.7/site-packages/django/core/handlers/base.py\", line 112, in get_response\n    response = wrapped_callback(request, *callback_args, **callback_kwargs)\n",
        "  File \"/usr/lib/python2.7/site-packages/django/views/generic/base.py\", line 69, in view\n    return self.dispatch(request, *args, **kwargs)\n",
        "  File \"/usr/lib/python2.7/site-packages/django/views/generic/base.py\", line 87, in dispatch\n    return handler(request, *args, **kwargs)\n",
        "  File \"/usr/lib/python2.7/site-packages/pulp/server/webservices/views/decorators.py\", line 237, in _auth_decorator\n    return _verify_auth(self, operation, super_user_only, method, *args, **kwargs)\n",
        "  File \"/usr/lib/python2.7/site-packages/pulp/server/webservices/views/decorators.py\", line 191, in _verify_auth\n    value = method(self, *args, **kwargs)\n",
        "  File \"/usr/lib/python2.7/site-packages/pulp/server/webservices/views/content.py\", line 373, in get\n    unit[constants.PULP_USER_METADATA_FIELDNAME])\n"
    ]
}

[root@ec2-54-78-44-32 ~]# curl -H "Accept: application/json" -H "WebFrameworkSwitch: django" -X PUT -k -u admin:admin -d {} 'https://localhost/pulp/api/v2/content/units/puppet_module/9011a43d-2cde-4b1f-a780-970c7eb6fca4/pulp_user_metadata/'|python -m json.tool
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100     6  100     4  100     2     10      5 --:--:-- --:--:-- --:--:--    10
null
[root@ec2-54-78-44-32 ~]# curl -H "Accept: application/json" -H "WebFrameworkSwitch: django" -X GET -k -u admin:admin 'https://localhost/pulp/api/v2/content/units/puppet_module/9011a43d-2cde-4b1f-a780-970c7eb6fca4/pulp_user_metadata/'|python -m json.tool
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100     2  100     2    0     0      5      0 --:--:-- --:--:-- --:--:--     5
{}

Randy, I am marking this as FAILED QE. Let me know if I understood something wrong.

Actions #28

Updated by rbarlow over 8 years ago

On 07/16/2015 06:54 AM, Pulp wrote:

Randy, I am marking this as FAILED QE. Let me know if I understood
something wrong.

We actually id this particular behavior on purpose. The thinking was
that {} is different than the data not existing, and the data not
existing indicates that nobody has ever put custom metadata on the
units. I had considered adding the custom metadata to all units as {},
but when discussing it with Michael we didn't think it was necessary.

--
Randy Barlow

Actions #29

Updated by ipanova@redhat.com over 8 years ago

After some conversation with Randy, we agreed that the behaviour should be consistent in all content types.
So the best option would be to have by default in all content types "pulp_user_metadata: {}"( this would also match what is described in docs)

Actions #32

Updated by ipanova@redhat.com over 8 years ago

  • Status changed from ASSIGNED to POST
  • Assignee changed from rbarlow to ipanova@redhat.com

Added by ipanova@redhat.com over 8 years ago

Revision 75df2fe3 | View on GitHub

As a user, I can add mutable custom metadata to a unit.

closes #86 https://pulp.plan.io/issues/86

Unit metadata dictionary needed to be updated and not replaced.

Actions #33

Updated by ipanova@redhat.com over 8 years ago

  • Status changed from POST to MODIFIED
Actions #35

Updated by dkliban@redhat.com over 8 years ago

  • Status changed from MODIFIED to 5
Actions #36

Updated by rbarlow about 8 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE
Actions #37

Updated by bmbouter about 6 years ago

  • Sprint set to January 2015
Actions #38

Updated by bmbouter about 6 years ago

  • Sprint/Milestone deleted (9)
Actions #39

Updated by bmbouter almost 5 years ago

  • Tags Pulp 2 added

Also available in: Atom PDF