#569 Better message for new-sources when file exists provided
Merged 2 years ago by onosek. Opened 2 years ago by drumian.
drumian/rpkg Issue_RHELCMP-5529  into  master

file modified
+18 -8
@@ -31,7 +31,9 @@ 

  from multiprocessing.dummy import Pool as ThreadPool

  from operator import itemgetter

  

+ import cccolutils

  import git

+ import koji

  import requests

  import rpm

  import six
@@ -39,11 +41,9 @@ 

  from six.moves import configparser, urllib

  from six.moves.urllib.parse import urljoin

  

- import cccolutils

- import koji

  from pyrpkg import layout

- from pyrpkg.errors import (HashtypeMixingError, UnknownTargetError,

-                            rpkgAuthError, rpkgError)

+ from pyrpkg.errors import (AlreadyUploadedError, HashtypeMixingError,

+                            UnknownTargetError, rpkgAuthError, rpkgError)

  from pyrpkg.lookaside import CGILookasideCache

  from pyrpkg.sources import SourcesFile

  from pyrpkg.utils import (cached_property, extract_srpm, find_me,
@@ -73,8 +73,9 @@ 

      specfile_uses_rpmautospec = None

  

  try:

+     from rpmautospec import \

+         calculate_release_number as rpmautospec_calculate_release_number

      from rpmautospec import process_distgit as rpmautospec_process_distgit

-     from rpmautospec import calculate_release_number as rpmautospec_calculate_release_number

  except ImportError:

      rpmautospec_process_distgit = None

      rpmautospec_calculate_release_number = None
@@ -2828,6 +2829,7 @@ 

          sourcesf = SourcesFile(self.sources_filename, self.source_entry_type,

                                 replace=replace)

          gitignore = GitIgnore(os.path.join(self.path, '.gitignore'))

+         all_files_already_uploaded = True

  

          for f in files:

              # TODO: Skip empty file needed?
@@ -2859,15 +2861,23 @@ 

                      "Hint: Use git for text files like the spec file, patches or helper scripts. "

                      "Use the lookaside cache for binary blobs, usually upstream source "

                      "tarballs.".format(f))

-             self.lookasidecache.upload(

-                 self.ns_repo_name if self.lookaside_namespaced else self.repo_name,

-                 f, file_hash, offline=offline)

+             try:

+                 self.lookasidecache.upload(

+                     self.ns_repo_name if self.lookaside_namespaced else self.repo_name,

+                     f, file_hash, offline=offline)

+             except AlreadyUploadedError:

+                 pass

+             else:

+                 all_files_already_uploaded = False

  

          sourcesf.write()

          gitignore.write()

  

          self.repo.index.add(['sources', '.gitignore'])

  

+         if all_files_already_uploaded:

+             raise AlreadyUploadedError('File already uploaded')

+ 

      def prep(self, arch=None, builddir=None, buildrootdir=None, define=None,

               extra_args=None):

          """Run ``rpmbuild -bp``

file modified
+14 -9
@@ -23,16 +23,17 @@ 

  import textwrap

  from gettext import gettext as _  # For `_ArgumentParser'

  

+ import koji_cli.lib

  import requests

  import rpm

  import six

  from requests.auth import HTTPBasicAuth

  from six.moves import configparser

  

- import koji_cli.lib

  import pyrpkg.utils as utils

- from pyrpkg import Modulemd

- from pyrpkg import rpkgError

+ from pyrpkg import Modulemd, rpkgError

+ 

+ from .errors import AlreadyUploadedError

  

  # argcomplete might not be available for all products which

  # use rpkg as a library (for example centpkg)
@@ -2552,12 +2553,16 @@ 

              if not os.path.isfile(file):

                  raise Exception('Path does not exist or is '

                                  'not a file: %s' % file)

-         self.cmd.upload(

-             self.args.files,

-             replace=self.args.replace,

-             offline=self.args.offline,)

-         self.log.info("Source upload succeeded. Don't forget to commit the "

-                       "sources file")

+         try:

+             self.cmd.upload(

+                 self.args.files,

+                 replace=self.args.replace,

+                 offline=self.args.offline,)

+         except AlreadyUploadedError:

+             self.log.info("All sources were already uploaded.")

+         else:

+             self.log.info("Source upload succeeded. Don't forget to commit the "

+                           "sources file")

  

      def upload(self):

          self.new_sources()

file modified
+5
@@ -67,3 +67,8 @@ 

  class LayoutError(rpkgError):

      """Raised when something went wrong while parsing/loading a layout"""

      pass

+ 

+ 

+ class AlreadyUploadedError(rpkgError):

+     """Raised when file is already uploaded"""

+     pass

file modified
+3 -2
@@ -24,7 +24,8 @@ 

  import six

  from six.moves import http_client

  

- from .errors import DownloadError, InvalidHashType, UploadError

+ from .errors import (AlreadyUploadedError, DownloadError, InvalidHashType,

+                      UploadError)

  

  

  class CGILookasideCache(object):
@@ -295,7 +296,7 @@ 

  

          if self.remote_file_exists(name, filename, hash):

              self.log.info("File already uploaded: %s", filepath)

-             return

+             raise AlreadyUploadedError('File already uploaded')

  

          self.log.info("Uploading: %s", filepath)

          post_data = [

@@ -6,9 +6,8 @@ 

  import tempfile

  

  from mock import patch

- from six.moves import StringIO

- 

  from pyrpkg.errors import rpkgError

+ from six.moves import StringIO

  

  from . import CommandTestCase

  

@@ -3,7 +3,6 @@ 

  import tempfile

  

  import git

- 

  import pyrpkg

  

  from . import CommandTestCase

file modified
+33 -4
@@ -1,6 +1,7 @@ 

  # -*- coding: utf-8 -*-

  

  import errno

+ import filecmp

  import glob

  import hashlib

  import logging
@@ -12,15 +13,16 @@ 

  import tempfile

  

  import git

+ import koji_cli.lib

  import six

+ import utils

  from mock import ANY, Mock, PropertyMock, call, mock_open, patch

  from six.moves import StringIO, configparser, http_client

+ from utils import CommandTestCase, FakeThreadPool

  

- import koji_cli.lib

  import pyrpkg.cli

- import utils

  from pyrpkg import Commands, Modulemd, layout, rpkgError

- from utils import CommandTestCase, FakeThreadPool

+ from pyrpkg.errors import AlreadyUploadedError

  

  try:

      import unittest2 as unittest
@@ -1229,10 +1231,17 @@ 

      def lookasidecache_upload(self, repo_name, filepath, hash, offline):

          filename = os.path.basename(filepath)

          storage_filename = os.path.join(self.lookasidecache_storage, filename)

+         if self._remote_file_exists(filepath, storage_filename):

+             raise AlreadyUploadedError("File already uploaded.")

          with open(storage_filename, 'wb') as fout:

              with open(filepath, 'rb') as fin:

                  fout.write(fin.read())

  

+     def _remote_file_exists(self, local_file, remote_file):

+         if os.path.isfile(remote_file):

+             return filecmp.cmp(local_file, remote_file)

+         return False

+ 

      def lookasidecache_download(self, name, filename, hash, outfile, hashtype=None, **kwargs):

          with open(outfile, 'w') as f:

              f.write('binary data')
@@ -1330,6 +1339,22 @@ 

                      r'.+README.rst which has different checksum.+',

                      cli.upload)

  

+     @patch('pyrpkg.log.info')

+     def test_upload_file_twice(self, log_info):

+         cli_cmd = [

+             'rpkg', '--path', self.cloned_repo_path, 'upload', self.readme_patch

+         ]

+ 

+         with patch('sys.argv', new=cli_cmd):

+             cli = self.new_cli()

+             with patch('pyrpkg.lookaside.CGILookasideCache.upload',

+                        new=self.lookasidecache_upload):

+                 cli.upload()

+                 cli.upload()

+ 

+         log_info.assert_has_calls(

+             call("All sources were already uploaded."))

+ 

  

  class TestSources(LookasideCacheMock, CliTestCase):

  
@@ -1503,7 +1528,11 @@ 

  

      def test_import(self):

          self.assert_import_srpm(self.chaos_repo)

-         self.assert_import_srpm(self.cloned_repo_path)

+         # Exception is not a functionality issue. There is no problem with

+         # uploading same file twice in a test however, the upload method checks

+         # if file has been already uploaded.

+         self.assertRaisesRegex(AlreadyUploadedError, r'File already uploaded',

+                                self.assert_import_srpm, self.cloned_repo_path)

  

      def test_import_gating_and_rpmlintrc_exception(self):

          # Add three additional files to the repo. Former gating.yaml and package.rpmlintrc are

file modified
+1 -1
@@ -12,8 +12,8 @@ 

  import rpm

  import six

  from mock import Mock, PropertyMock, call, mock_open, patch

- 

  from pyrpkg import rpkgError

+ 

  from utils import CommandTestCase

  

  

file modified
+1 -1
@@ -4,8 +4,8 @@ 

  

  import requests

  from mock import Mock, patch

- 

  from pyrpkg import Modulemd

+ 

  from utils import CommandTestCase

  

  try:

file modified
+6 -3
@@ -15,8 +15,8 @@ 

  

  import mock

  import pycurl

- 

- from pyrpkg.errors import DownloadError, InvalidHashType, UploadError

+ from pyrpkg.errors import (AlreadyUploadedError, DownloadError,

+                            InvalidHashType, UploadError)

  from pyrpkg.lookaside import CGILookasideCache

  

  old_stat = os.stat
@@ -510,7 +510,10 @@ 

          hash = 'thehash'

  

          with mock.patch.object(lc, 'remote_file_exists', lambda *x: True):

-             lc.upload('pyrpkg', 'pyrpkg-0.0.tar.xz', hash)

+             # self.assertRaises(AlreadyUploadedError, lc.upload, 'pyrpkg',

+             #                  'pyrpkg-0.tar.xz', hash)

+             self.assertRaisesRegex(AlreadyUploadedError, r'File already uploaded',

+                                    lc.upload, 'pyrpkg', 'pyrpkg-0.tar.xz', hash)

  

          self.assertEqual(curl.perform.call_count, 0)

          self.assertEqual(curl.setopt.call_count, 0)

file modified
+2 -3
@@ -6,11 +6,10 @@ 

  import tempfile

  

  import mock

- import six

- from six.moves import configparser

- 

  import pyrpkg.cli

+ import six

  from pyrpkg.errors import rpkgError

+ from six.moves import configparser

  

  # For running tests with Python 2

  try:

file modified
+2 -2
@@ -3,12 +3,12 @@ 

  import logging

  import os

  

+ import koji

  import mock

+ import pyrpkg.cli

  import six

  from six.moves import StringIO, configparser

  

- import koji

- import pyrpkg.cli

  from utils import CommandTestCase

  

  if six.PY2:

file modified
+1 -1
@@ -4,9 +4,9 @@ 

  import warnings

  

  import mock

- 

  from pyrpkg.utils import (cached_property, is_file_in_directory,

                            is_file_tracked, log_result, warn_deprecated)

+ 

  from utils import CommandTestCase

  

  

file modified
-1
@@ -7,7 +7,6 @@ 

  import tempfile

  

  import six

- 

  from pyrpkg import Commands

  

  # For running tests with Python 2

Fixes the bug when new-sources prints our the message "Source upload succeeded. Don't forget to commit the sources file" when file is already uploaded (so there is no upload).

Fixes: #533
JIRA: RHELCMP-5529

Signed-off-by: Dominik Rumian drumian@redhat.com

rebased onto 145f6c8d37a32fc8a396bac60cabb4b46f1da518

2 years ago

rebased onto 7c46cff03c653352c69b9fb777fdad148bdc79ff

2 years ago

rebased onto 929c62a9aff8e43b96a849f859b97ed74ae63b78

2 years ago

rebased onto a8a732bc900cf590886adc6953793517a6fcb6f3

2 years ago

rebased onto 8020ea43adfa461cb9b4a0daa45f20bd8122cc52

2 years ago

rebased onto 9553a5a9b35eccdc39591b854d8dcf646fba2ec9

2 years ago

rebased onto 55caa85

2 years ago

Looks good. Congratulation on your first rpkg contribution.

Pull-Request has been merged by onosek

2 years ago