Issue #2500
closed
ValueError exception is raised when uploading module with invalid name
Added by mhrivnak over 7 years ago. Updated about 5 years ago.
Description
When uploading a puppet module that has an unparsable name, pulp raises a ValueError, which bubbles up and results in:
PLP0047: The importer puppet_importer indicated a failed response when uploading puppet_module unit to repository
Pulp should raise a PulpCodedException with a more helpful error message.
From Bugzilla:
The underlying issue appears to be:
Oct 31 16:21:57 ndhrhnsat2 pulp: pulp_puppet.plugins.importers.importer:ERROR: (17600-26528) File "/usr/lib/python2.7/site-packages/pulp_puppet/plugins/db/models.py", line 166, in split_filename Oct 31 16:21:57 ndhrhnsat2 pulp: pulp_puppet.plugins.importers.importer:ERROR: (17600-26528) author, name = filename.split("/", 1) Oct 31 16:21:57 ndhrhnsat2 pulp: pulp_puppet.plugins.importers.importer:ERROR: (17600-26528) ValueError: need more than 1 value to unpack
Models.py does not seem to like name values in the manifest that use underscores:
For example, changing
"name": "eodendahl_ssh",
to:
"name": "eodendahl-ssh",
bypasses the error.
Related issues
Updated by bizhang over 7 years ago
- Sprint/Milestone set to 31
- Tags Easy Fix added
Updated by daviddavis over 7 years ago
The PLP0047 error is what I get even if the exception is properly handled currently in the database. Below is an example where I uploaded an invalid tarball which we handle in the code here:
$ pulp-admin puppet repo create --repo-id blah
$ touch blah-blah-1.2.1.tar.gz
$ pulp-admin puppet repo uploads upload -f blah-blah-1.2.1.tar.gz --repo-id blah
+----------------------------------------------------------------------+
Unit Upload
+----------------------------------------------------------------------+
Extracting necessary metadata for each request...
[==================================================] 100%
Analyzing: blah-blah-1.2.1.tar.gz
... completed
Creating upload requests on the server...
[==================================================] 100%
Initializing: blah-blah-1.2.1.tar.gz
... completed
Starting upload of selected units. If this process is stopped through ctrl+c,
the uploads will be paused and may be resumed later using the resume command or
canceled entirely using the cancel command.
Uploading: blah-blah-1.2.1.tar.gz
... completed
Importing into the repository...
This command may be exited via ctrl+c without affecting the request.
[\]
Running...
Task Failed
The importer puppet_importer indicated a failed response when uploading
puppet_module unit to repository blah.
Deleting the upload request...
... completed
Here's what journalctl looks like and it seems correct:
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) Invalid properties: [u'/var/lib/pulp/uploads/d99efe05-f4d1-47d3-b4f4-e03d04fc7ce9']
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) Traceback (most recent call last):
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) File "/home/vagrant/devel/pulp_puppet/pulp_puppet_plugins/pulp_puppet/plugins/importers/importer.py", line 82, in upload_unit
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) conduit)
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) File "/home/vagrant/devel/pulp_puppet/pulp_puppet_plugins/pulp_puppet/plugins/importers/upload.py", line 50, in handle_uploaded_unit
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) extracted_data = metadata_parser.extract_metadata(file_path, repo.working_dir)
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) File "/home/vagrant/devel/pulp_puppet/pulp_puppet_plugins/pulp_puppet/plugins/importers/metadata.py", line 59, in extract_metadata
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) metadata = _extract_json(filename, temp_dir)
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) File "/home/vagrant/devel/pulp_puppet/pulp_puppet_plugins/pulp_puppet/plugins/importers/metadata.py", line 104, in _extract_json
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) tgz = tarfile.open(name=filename)
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) File "/usr/lib64/python2.7/tarfile.py", line 1678, in open
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) raise ReadError("file could not be opened successfully")
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp_puppet.plugins.importers.importer:ERROR: (4747-26752) InvalidTarball: Invalid properties: [u'/var/lib/pulp/uploads/d99efe05-f4d1-47d3-b4f4-e03d04fc7ce9']
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp.server.managers.content.upload:ERROR: (4747-26752) Error from the importer while importing uploaded unit to repository [saz]
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp.server.managers.content.upload:ERROR: (4747-26752) Traceback (most recent call last):
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp.server.managers.content.upload:ERROR: (4747-26752) File "/home/vagrant/devel/pulp/server/pulp/server/managers/content/upload.py", line 223, in import_uploaded_unit
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp.server.managers.content.upload:ERROR: (4747-26752) unit_type=unit_type_id, summary=result['summary'], details=result['details']
Jan 03 20:10:03 dev.example.com pulp[4747]: pulp.server.managers.content.upload:ERROR: (4747-26752) PulpCodedException: The importer puppet_importer indicated a failed response when uploading puppet_module unit to repository saz.
Is this the correct behavior? Or should we be showing something more useful to the user? I ask because I am planning on making a similar change in which we'll be raising a similar coded pulp error.
Updated by bmbouter over 7 years ago
+1 to doing something more useful for the user.
In terms of making that change, I think @dkliban can give good advice in this area. If you have specific questions mhrivnak or I could also give some advice.
Added by daviddavis over 7 years ago
Updated by daviddavis over 7 years ago
Talked with @dkilban and we opened a new issue to handle how error messages during uploads are displayed to the user as it's a problem that predates this current bug, it's going to be a bigger effort, and there might be some backwards compatibility issues:
Updated by daviddavis over 7 years ago
- Status changed from ASSIGNED to POST
Updated by daviddavis over 7 years ago
- Related to Issue #2512: Puppet Importer swallows exception when one is raised during upload added
Updated by daviddavis over 7 years ago
- Subject changed from Error message is generic when uploading module with invalid name to ValueError exception is raised when uploading module with invalid name
Updated by daviddavis over 7 years ago
- Status changed from POST to MODIFIED
Applied in changeset a7adae36e79d13a3e825e1e0e8f35d7b374ab712.
Added by daviddavis over 7 years ago
Revision 39c63f22 | View on GitHub
Capture ValueError and raise a more informative error
Updated by daviddavis over 7 years ago
Applied in changeset 39c63f22e992a123fe36bd1b4438354369e274c4.
Updated by semyers over 7 years ago
daviddavis@redhat.com wrote:
Applied in changeset a7adae36e79d13a3e825e1e0e8f35d7b374ab712.
This is the commit from the PR referenced in an earlier comment, which was merged to master.
Applied in changeset 39c63f22e992a123fe36bd1b4438354369e274c4.
This is a cherry-pick of the a7adae commit back to 2.11-dev, which is how this change becomes available in 2.11.1.
Updated by pthomas@redhat.com over 7 years ago
- Status changed from 5 to 6
verified
[root@qe-blade-05 ~]# pulp-admin puppet repo create --repo-id blah
Successfully created repository [blah]
[root@qe-blade-05 ~]# touch blah-blah-1.2.1.tar.gz
[root@qe-blade-05 ~]# pulp-admin puppet repo uploads upload -f blah-blah-1.2.1.tar.gz --repo-id blah
+----------------------------------------------------------------------+
Unit Upload
+----------------------------------------------------------------------+
Extracting necessary metadata for each request...
[==================================================] 100%
Analyzing: blah-blah-1.2.1.tar.gz
... completed
Creating upload requests on the server...
[==================================================] 100%
Initializing: blah-blah-1.2.1.tar.gz
... completed
Starting upload of selected units. If this process is stopped through ctrl+c,
the uploads will be paused and may be resumed later using the resume command or
canceled entirely using the cancel command.
Uploading: blah-blah-1.2.1.tar.gz
... completed
Importing into the repository...
This command may be exited via ctrl+c without affecting the request.
[\]
Running...
Task Failed
The importer puppet_importer indicated a failed response when uploading
puppet_module unit to repository blah.
Deleting the upload request...
... completed
Updated by semyers over 7 years ago
- Status changed from 6 to CLOSED - CURRENTRELEASE
Updated by daviddavis about 7 years ago
To clarify the change here is that we're now raising an InvalidModuleName error instead of a ValueError error. This should be visible in the logs. The end user should see this change in the CLI/API after https://pulp.plan.io/issues/2512 is fixed.
.
Capture ValueError and raise a more informative error
fixes #2500 https://pulp.plan.io/issues/2500