#394 Add '_pkgname_' placeholder in config to tell fedora-create-review to create new folder to upload spec/srpm to
Closed 3 years ago by ngompa. Opened 3 years ago by ankursinha.
ankursinha/FedoraReview ticket-283  into  master

@@ -54,6 +54,8 @@ 

  

  Description:

  %s

+ 

+ Fedora Account System Username: %s

  """

  

  # Initial simple logging stuff
@@ -130,7 +132,9 @@ 

      """ gitsync Settings """

  

      # upload target

-     upload_target = "fedorapeople.org:public_html/"

+     # @pkgname@ is replaced by the name of the package

+     pkgname_placeholder = "@pkgname@/"

+     upload_target = "fedorapeople.org:public_html/" + pkgname_placeholder

  

      def __init__(self):

          """Constructor of the Settings object.
@@ -213,7 +217,7 @@ 

                  review_type, self.info["name"], self.info["summary"]

              ),

              "comment": BUG_COMMENT

-             % (self.info["specurl"], self.info["srpmurl"], self.info["description"]),

+             % (self.info["specurl"], self.info["srpmurl"], self.info["description"], self.username),

              "rep_platform": "Unspecified",

              "bug_severity": "unspecified",

              "op_sys": "Unspecified",
@@ -258,6 +262,12 @@ 

          info in the settings.

          """

          complement_url = self.settings.upload_target.split("public_html/")[1]

+ 

+         # If the URL ends with @pkgname@, replace it with the package name

+         if complement_url.endswith(self.settings.pkgname_placeholder):

+             package_name = self.retrieve_name()

+             complement_url = complement_url.replace(self.settings.pkgname_placeholder, package_name)

+ 

          url = "https://{}.fedorapeople.org/{}/".format(self.username, complement_url)

          self.info["specurl"] = url + os.path.basename(self.specfile)

          self.info["srpmurl"] = url + os.path.basename(self.srpmfile)
@@ -361,8 +371,33 @@ 

      def upload_files(self):

          """ Upload the spec file and the src.rpm files into

          fedorapeople.org, ensuring readable mode."""

+ 

          print("Uploading files into fedorapeople")

-         self.log.debug("Target: %s", self.settings.upload_target)

+         upload_url = self.settings.upload_target

+         if upload_url.endswith(self.settings.pkgname_placeholder):

+             package_name = self.retrieve_name()

+             upload_url = upload_url.replace(self.settings.pkgname_placeholder, package_name) + "/"

+ 

+             dir_cmd = [

+                 "ssh",

+                 self.username + "@fedorapeople.org",

+                 "mkdir -p ~/public_html/{}".format(package_name)

This ignores an invidual setting in config file, e.g. another subfolder.

+             ]

+             self.log.debug(dir_cmd)

+             try:

+                 proc = Popen(

+                     dir_cmd,

+                     stdout=subprocess.PIPE,

+                     stderr=subprocess.PIPE,

+                     universal_newlines=True,

+                 )

+                 output = proc.communicate()[0]

+             except OSError as err:

+                 print("OSError : %s" % str(err))

+                 return (output, proc.returncode)

+ 

+         self.log.debug("Target: %s", upload_url)

+ 

          for path in [self.specfile, self.srpmfile]:

              mode = os.stat(path)[0]

              mode |= stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH
@@ -371,7 +406,7 @@ 

              "scp",

              self.specfile,

              self.srpmfile,

-             self.username + "@" + self.settings.upload_target,

+             self.username + "@" + upload_url

          ]

          self.log.debug(cmd)

          try:

Also adds FAS to review comment

wouldn't this make sense as something like @pkgname@/ instead of with underscores?

Sure, i didnt really know what sentinels to use. Any other changes?

--
Thanks,

Ankur

(Sent from mobile device)

The rest of it looks fine, I just need to test it. :wink:

1 new commit added

  • create-review: Use @ instead of _ as sentinel
3 years ago

This ignores an invidual setting in config file, e.g. another subfolder.

This ignores an invidual setting in config file, e.g. another subfolder.

The folder was not created automatically before, and this PR does not add that functionality. It is only adds the functionality of creating a new folder for each package when the placeholder is used. Generally creating a new folder will require a different PR (and is not in the scope of this PR).

@ankursinha are you still planning to work on this? I'd love to see this feature merged, and was about to start coding it up myself before I saw this PR.

This ignores an invidual setting in config file, e.g. another subfolder.

The folder was not created automatically before, and this PR does not add that functionality. It is only adds the functionality of creating a new folder for each package when the placeholder is used. Generally creating a new folder will require a different PR (and is not in the scope of this PR).

I think @raphgro's point is that this should respect upload_target instead of hardcoding public_html.

@dcavalca : please feel free to work on this, I'm not going to make any further improvements here. Cheers.

Thanks! I put up #402 to replace this.

Pull-Request has been closed by ngompa

3 years ago
Metadata