Updated patch attached, with some comments from Michal incorporated.
Again this needs to be applied on top of Shawn's patch:
http://cr.opensolaris.org/~swalker/pkg-11522/
JR
John Rice wrote:
Shawn, Joanie, Michal and Takeshi,
I have put together a patch (attached) which makes it a lot easier for
users to add a publisher in the GUI and should significantly reduce
the instances where the UnknownRepositoryPublishers exception error
is displayed to the user. So its now much less urgent to have the
error translated, though it would be nice to of course.
When a user adds a publisher if the name given (e.g. contrib) is not
known by the publisher, then the first known name for the publisher
(e.g. contrib.opensolaris.org) is used to add the publisher and the
name given by the user (e.g. contrib) is set to the alias for this
publisher. The successfully added publisher dialog then shows for
example:
Publisher added successfully
-------------------------------------------------
Publisher Name: contrib (contrib.opensolaris.org)
URI: http://pkg.opensolaris.org/contrib
So the user knows exactly what's happened.
The attached patch should be put on top of Shawns 11522 patch.
http://cr.opensolaris.org/~swalker/pkg-11522/
Here's a combined webrev for those who want it:
http://cr.opensolaris.org/~jmr/11522_gui_pub_changes_wip_5Feb_1240pm/
Comments below on the different scenarios Shawn raised.
JR
P.s. Joaine, I don't think you can trigger this from the modify dialog
anymore, as you will always have a valid pub added in the first place,
but feel free to show me the errors of my ways :)
Not quite true, there are a few scenarios in which you would still
have to error in some fashion:
* a publisher already exists with one of the 'known' prefixes
Have pub:
opensolaris.org
http://pkg.opensolaris.org
Add:
oss
http://pkg.opensolaris.org
Get: Successfully Added Dialog
oss (opensolaris.org)
http://pkg.opensolaris.org
So have just changed the alias, we could inform the user that that's
what's happened but not without string changes which we want to avoid.
* a publisher already exists with an alias the same as one of the
'known' prefixes
Catch this when user attempts to put in the name:
Have pub:
opensolaris.org
http://pkg.opensolaris.org
Type in:
opensolaris.org
Get red error string below field:
"Name already in use"
* there's no publisher configuration information all (the repository
is misconfigured)
If no known pubs are returned in UnknownRepositoryPublishers the error
gets displayed. I imagine this will be the exception rather than the
rule if you pardon the pun.
To be clear, I have no problem with making the 'known', 'unknown',
and 'location' attributes accessible from the exception object itself.
The above patch does this.
If you want to put additional logic into place, that's the perogative
of the client. I'll do that tomorrow.
Cheers,
------------------------------------------------------------------------
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
# This patch file was generated by NetBeans IDE
# Following Index: paths are relative to:
/export/home/jr140578/work/11522_gate/src
# This patch can be applied using context Tools: Patch action on respective
folder.
# It uses platform neutral UTF-8 encoding and \n newlines.
# Above lines and this line are ignored by the patching process.
Index: gui/modules/repository.py
--- gui/modules/repository.py Base (BASE)
+++ gui/modules/repository.py Locally Modified (Based On LOCAL)
@@ -553,9 +553,10 @@
if not misc.valid_pub_prefix(name):
self.name_error = _("Name contains invalid characters")
return False
- if name in [p.prefix for p in self.api_o.get_publishers()]:
- self.name_error = _("Name already in use")
- return False
+ for p in self.api_o.get_publishers():
+ if name == p.prefix or name == p.alias:
+ self.name_error = _("Name already in use")
+ return False
return True
def __get_selected_publisher_itr_model(self):
@@ -830,7 +831,19 @@
new_pub = True
name = pub.prefix
errors_ssl = self.__update_ssl_creds(pub, repo, ssl_cert,
ssl_key)
- errors_update = self.__update_publisher(pub,
new_publisher=new_pub)
+ errors_update = []
+ try:
+ errors_update = self.__update_publisher(pub,
+ new_publisher=new_pub)
+ except api_errors.UnknownRepositoryPublishers, e:
+ if len(e.known) > 0:
+ pub, repo, new_pub =
self.__get_or_create_pub_with_url(
+ self.api_o, e.known[0], origin_url)
+ pub.alias = name
+ errors_update = self.__update_publisher(pub,
+ new_publisher=new_pub,
raise_unknownpubex=False)
+ else:
+ errors_update.append((pub, e))
errors += errors_ssl
errors += errors_update
if self.cancel_progress_thread:
@@ -861,7 +874,7 @@
self.w_add_publisher_dialog, None)
self.parent.reload_packages()
- def __update_publisher(self, pub, new_publisher=False):
+ def __update_publisher(self, pub, new_publisher=False,
raise_unknownpubex=True):
errors = []
try:
self.no_changes += 1
@@ -879,6 +892,11 @@
else:
self.__g_update_details_text(
_("Publisher %s succesfully updated\n") %
pub.prefix)
+ except api_errors.UnknownRepositoryPublishers, e:
+ if raise_unknownpubex:
+ raise e
+ else:
+ errors.append((pub, e))
except api_errors.CatalogRefreshException, e:
errors.append((pub, e))
except api_errors.InvalidDepotResponseException, e:
@@ -896,7 +914,11 @@
# Descriptions not available at the moment
self.w_add_publisher_c_desc.hide()
self.w_add_publisher_c_desc_l.hide()
- self.w_add_publisher_c_name.set_text(pub.prefix)
+ if pub.alias and len(pub.alias) > 0:
+ self.w_add_publisher_c_name.set_text(
+ pub.alias + " ("+ pub.prefix +")")
+ else:
+ self.w_add_publisher_c_name.set_text(pub.prefix)
self.w_add_publisher_c_url.set_text(origin.uri)
self.w_add_publisher_comp_dialog.show()
@@ -1099,7 +1121,10 @@
repo = pub.selected_repository
pub.alias = alias
errors += self.__update_ssl_creds(pub, repo, ssl_cert, ssl_key)
- errors += self.__update_publisher(pub, new_publisher=False)
+ try:
+ errors += self.__update_publisher(pub,
new_publisher=False)
+ except api_errors.UnknownRepositoryPublishers, e:
+ errors.append((pub, e))
self.progress_stop_thread = True
if len(errors) > 0:
gobject.idle_add(self.__show_errors, errors)
Index: modules/client/api_errors.py
--- modules/client/api_errors.py Base (BASE)
+++ modules/client/api_errors.py Locally Modified (Based On LOCAL)
@@ -1176,37 +1176,37 @@
class UnknownRepositoryPublishers(PublisherError):
"""Used to indicate that one or more publisher prefixes are unknown by
the specified repository."""
+ def __init__(self, origins=EmptyI, location=EmptyI, known=EmptyI,
unknown=EmptyI):
+ PublisherError.__init__(self)
+ self.origins = origins
+ self.location = location
+ self.known = known
+ self.unknown = unknown
def __str__(self):
- if "location" in self._args:
- return _("""
-The repository at the location listed below does not contain
-package data for %(unknown)s; only %(known)s:
+ if len(self.location) > 0:
+ return _(
+"The repository at the location listed below does not contain "
+"package data for %(unknown)s; only %(known)s:\n\n%(location)s\n\n"
+"This is either because the repository location that was provided "
+"is not valid, or because the provided publisher information does "
+"not match that known by the repository.") % {
+ "unknown": ", ".join(self.unknown),
+ "location": self.location,
+ "known": ", ".join(self.known) }
-%(location)s
+ if len(self.origins) > 0:
+ return _(
+"One or more of the repository origin(s) listed below contains package "
+"data for %(known)s; not %(unknown)s:\n\n%(origins)s\n\n"
+"This is either because one of the repository origins is not valid for "
+"this publisher, or because the publisher configuration retrieved from "
+"the repository origin does not match the client configuration.") % {
+ "unknown": self.unknown[0],
+ "known": ", ".join(self.known),
+ "origins": "\n".join(str(o) for o in self.origins) }
-This is either because the repository location that was provided
-is not valid, or because the provided publisher information does
-not match that known by the repository.
-""") % { "unknown": ", ".join(self._args["unknown"]),
- "location": self._args["location"],
- "known": ", ".join(self._args["known"]) }
- if "origins" in self._args:
- return _("""
-One or more of the repository origin(s) listed below contains package
-data for %(known)s; not %(unknown)s:
-
-%(origins)s
-
-This is either because one of the repository origins is not valid for
-this publisher, or because the publisher configuration retrieved from
-the repository origin does not match the client configuration.
-""") % { "unknown": self._args["unknown"][0],
- "known": ", ".join(self._args["known"]),
- "origins": "\n".join(str(o) for o in self._args["origins"]) }
-
-
class UnknownRelatedURI(PublisherError):
"""Used to indicate that no matching related URI could be found using
the provided criteria."""
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss