Story #86
closed
As a user, I can add mutable custom metadata to a unit
Added by mhrivnak over 9 years ago. Updated almost 3 years ago.
100%
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
Updated by cduryee over 9 years ago
Would this blob ever be synced down from a feed url or published?
Updated by bcourt over 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?
Updated by mhrivnak over 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.
Updated by rbarlow over 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.
Updated by cduryee over 9 years ago
will there be enforcement of how much data can be stored in there?
Updated by rbarlow over 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?
Updated by cduryee over 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:)
Updated by rbarlow over 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!
Updated by rbarlow over 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.
Updated by rbarlow over 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.
Updated by rbarlow over 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.
Updated by rbarlow over 9 years ago
- Status changed from ASSIGNED to POST
- % Done changed from 60 to 90
Added by rbarlow over 9 years ago
Added by rbarlow over 9 years ago
Updated by rbarlow over 9 years ago
- Status changed from POST to MODIFIED
- % Done changed from 90 to 100
Applied in changeset pulp|commit:8564bb935e029203ae739f61166b1d4eed88dc07.
Added by rbarlow over 9 years ago
Revision a7cc9404 | View on GitHub
Fix broken unit tests due to a change in platform.
re #86
Updated by rbarlow over 9 years ago
I also needed to make this change in pulp_rpm to respond to these changes:
Updated by rbarlow about 9 years ago
- Platform Release changed from master to 2.7.0
Updated by dkliban@redhat.com almost 9 years ago
- Status changed from MODIFIED to 5
Updated by ipanova@redhat.com almost 9 years ago
- Groomed set to No
- Sprint Candidate set to No
Updated by ipanova@redhat.com almost 9 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.
Updated by rbarlow almost 9 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
Updated by ipanova@redhat.com almost 9 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)
Updated by ipanova@redhat.com over 8 years ago
- Status changed from ASSIGNED to POST
- Assignee changed from rbarlow to ipanova@redhat.com
Fixed the pulp_puppet plugin https://github.com/pulp/pulp_puppet/pull/192
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.
Updated by ipanova@redhat.com over 8 years ago
- Status changed from POST to MODIFIED
Applied in changeset pulp_puppet:75df2fe373152f7d51e9cce4b3737cda27df85bf.
Updated by dkliban@redhat.com over 8 years ago
- Status changed from MODIFIED to 5
Updated by rbarlow about 8 years ago
- Status changed from 5 to CLOSED - CURRENTRELEASE
.
Add an API for storing custom Unit metadata.
https://pulp.plan.io/issues/86
closes #86