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

fixes #19063 - uploads packages with correct name #6813

Merged
merged 1 commit into from
Jun 9, 2017

Conversation

parthaa
Copy link
Contributor

@parthaa parthaa commented Jun 6, 2017

This commit fixes an issue with the package uploads
The prior logic includes the package name for yum repos
while excludes the name for file repos. What we intend to
happen is the reverse. We need to include the name for file
but exclude for yum since the name is derived from the metadata
for yum, puppet and other types.

This commit also contains a couple of unit tests to handle this.

@@ -850,6 +850,37 @@ def test_import_uploads
assert_response :success
end

def test_import_uploads

Choose a reason for hiding this comment

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

Method Katello::Api::V2::RepositoriesControllerTest#test_import_uploads is defined at both test/controllers/api/v2/repositories_controller_test.rb:840 and test/controllers/api/v2/repositories_controller_test.rb:853.

upload.except('id').except('name')
else
upload.except('id')
end
end


Choose a reason for hiding this comment

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

Extra blank line detected.

@@ -365,13 +366,14 @@ def import_uploads

upload_ids = uploads.map { |upload| upload['id'] }
unit_keys = uploads.map do |upload|
if @repository.file?
unless @repository.file?

Choose a reason for hiding this comment

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

Do not use unless with else. Rewrite these with the positive case first.

@parthaa parthaa force-pushed the package-upload branch 2 times, most recently from dc51ecc to e463548 Compare June 6, 2017 20:29
This commit fixes an issue with the package uploads
The prior logic includes the package name for yum repos
while excludes the name for file repos. What we intend to
happen is the reverse. We need to include the name for file
but exclude for yum since the name is derived from the metadata
for yum, puppet and other types.

This commit also contains a couple of unit tests to handle this.
@johnpmitsch
Copy link
Contributor

package uploaded with hammer repository upload-content --product "prod1" --name "walrus" --organization-id 1 --path ~/walrus-5.21-1.noarch.rpm

on master

[vagrant@koji hammer-cli-katello{master}]$ hammer package info --id 1                                                                                               No permissions to create log dir /var/log/hammer
File /var/log/hammer/hammer.log not writeable, won't log anything to the file!
ID:            1
Name:          walrus-5.21-1.noarch.rpm
Version:       5.21
Architecture:  noarch
Epoch:         0
Release:       1
Author:
Filename:      walrus-5.21-1.noarch.rpm
Build Host:    dhcp-27-190.brq.redhat.com
Vendor:
License:       GPLv2
Relative Path: walrus-5.21-1.noarch.rpm
Description:   A dummy package of walrus

on PR branch

No permissions to create log dir /var/log/hammer
File /var/log/hammer/hammer.log not writeable, won't log anything to the file!
ID:            2
Name:          walrus
Version:       5.21
Architecture:  noarch
Epoch:         0
Release:       1
Author:
Filename:      walrus-5.21-1.noarch.rpm
Build Host:    dhcp-27-190.brq.redhat.com
Vendor:
License:       GPLv2
Relative Path: walrus-5.21-1.noarch.rpm
Description:   A dummy package of walrus

The name is corrected by this change 👍

@parthaa parthaa merged commit b6bab83 into Katello:master Jun 9, 2017
@parthaa parthaa deleted the package-upload branch June 9, 2017 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants