Hi Joe,

GitHub itself moved to requiring the new API several months back, and for
that, we had to introduce deeper hosting service support. In order to talk
to github.com, Hosting Service must be set to GitHub.

Now, for a self-hosted GitHub instance, I imagine the old API still exists.
In that case, you'll have to keep the raw file URL field, as that's how
Review Board fetched files and checked for file existence with the old API.
Bringing that back may solve your problem.

We have a contribution we'll be incorporating (once it's ready) for Hosting
Service support for self-hosted GitHub instances. We'll release that when
it's ready. It should then be a bit easier to get going with the new API.

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Fri, Oct 5, 2012 at 8:59 AM, Joe <jfris...@globys.com> wrote:

> Hi,
>
> We upgraded to 1.6.12 from 1.6.3 yesterday. We're using an internal github
> as our master repository, and have about 20 repositories defined in
> reviewboard. The hosting service is set to None, the type to git, the paths
> look
> like g...@github.corp.ad.local:cm/transform.git. For the upgrade we
> deleted the raw file URL mask and added a username & password (that is we
> wanted to move to the new git hub api). Everything seemed to work, we could
> add a small test file and post the review for the repositories, but then we
> discovered it wouldn't work for read diffs. We've fiddled with everything
> we could think of to no avail -- any help would be appreciated.  Here's an
> example of review that worked (with a failing post below):
>
> $ post-review --debug
> >>> RBTools 0.4.2
> >>> Python 2.6.6 (r266:84292, Dec  7 2011, 20:38:36)
> [GCC 4.4.6 20110731 (Red Hat 4.4.6-3)]
> >>> Running on Linux-2.6.32-220.13.1.el6.i686-i686-with-centos-6.2-Final
> >>> Home = /home/CORP/jfrisbie
> >>> Current Directory = /home/CORP/jfrisbie/projects/transform
> >>> Checking the repository type. Errors shown below are mostly harmless.
> DEBUG:root:Checking for a CVS repository...
> DEBUG:root:Checking for a ClearCase repository...
> DEBUG:root:Checking for a Git repository...
> DEBUG:root:Running: git rev-parse --git-dir
> DEBUG:root:Running: git config core.bare
> DEBUG:root:Running: git rev-parse --show-toplevel
> DEBUG:root:Running: git symbolic-ref -q HEAD
> DEBUG:root:Running: git config --get branch.master.merge
> DEBUG:root:Running: git config --get branch.master.remote
> DEBUG:root:Running: git config --get remote.origin.url
> DEBUG:root:repository info: Path: g...@github.corp.ad.local:cm/transform.git,
> Base path: , Supports changesets: False
> >>> Finished checking the repository type.
> DEBUG:root:Running: git config --get reviewboard.url
> >>> HTTP GETting api/
> >>> HTTP GETting https://reviewboard.corp.ad.local/api/info/
> >>> Using the new web API
> DEBUG:root:Running: git merge-base origin/master refs/heads/master
> DEBUG:root:Running: git diff --no-color --full-index --no-ext-diff
> --ignore-submodules
> 08d87f9f8aa11cf60d2f6419fb9fe43d4d336967..refs/heads/master
> DEBUG:root:Running: git log --pretty=format:%s HEAD^..
> DEBUG:root:Running: git log --pretty=format:%s%n%n%b
> 08d87f9f8aa11cf60d2f6419fb9fe43d4d336967..
> >>> Attempting to create review request on 
> >>> g...@github.corp.ad.local:cm/transform.git
> for None
> >>> HTTP POSTing to https://reviewboard.corp.ad.local/api/review-requests/:
> {'repository': 'g...@github.corp.ad.local:cm/transform.git'}
> >>> Review request created
> >>> Attempting to set field 'target_groups' to 'PDDev' for review request
> '405'
> >>> HTTP PUTting to
> https://reviewboard.corp.ad.local/api/review-requests/405/draft/:
> {'target_groups': 'PDDev'}
> >>> Attempting to set field 'summary' to 'With Master' for review request
> '405'
> >>> HTTP PUTting to
> https://reviewboard.corp.ad.local/api/review-requests/405/draft/:
> {'summary': 'With Master'}
> >>> Attempting to set field 'description' to 'With Master' for review
> request '405'
> >>> HTTP PUTting to
> https://reviewboard.corp.ad.local/api/review-requests/405/draft/:
> {'description': 'With Master'}
> >>> Uploading diff, size: 205
> >>> HTTP POSTing to
> https://reviewboard.corp.ad.local/api/review-requests/405/diffs/: {}
> Review request #405 posted.
>
> https://reviewboard.corp.ad.local/r/405/
>
> $ post-review --output-diff
> diff --git a/Master.txt b/Master.txt
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..1796c7667d90e91a02868ea565c3b6aedea5e335
> --- /dev/null
> +++ b/Master.txt
> @@ -0,0 +1 @@
> +Yes, Master
>
>
> Here's an example of a review that fails -- same local git, different
> branch checked out with a real change (both branches have one diff on top
> of what's at the tip of the master repo:
>
> $ post-review --debug
> >>> RBTools 0.4.2
> >>> Python 2.6.6 (r266:84292, Dec  7 2011, 20:38:36)
> [GCC 4.4.6 20110731 (Red Hat 4.4.6-3)]
> >>> Running on Linux-2.6.32-220.13.1.el6.i686-i686-with-centos-6.2-Final
> >>> Home = /home/CORP/jfrisbie
> >>> Current Directory = /home/CORP/jfrisbie/projects/transform
> >>> Checking the repository type. Errors shown below are mostly harmless.
> DEBUG:root:Checking for a CVS repository...
> DEBUG:root:Checking for a ClearCase repository...
> DEBUG:root:Checking for a Git repository...
> DEBUG:root:Running: git rev-parse --git-dir
> DEBUG:root:Running: git config core.bare
> DEBUG:root:Running: git rev-parse --show-toplevel
> DEBUG:root:Running: git symbolic-ref -q HEAD
> DEBUG:root:Running: git config --get branch.working.merge
> DEBUG:root:Running: git config --get branch.working.remote
> DEBUG:root:Running: git config --get remote.origin.url
> DEBUG:root:repository info: Path: g...@github.corp.ad.local:cm/transform.git,
> Base path: , Supports changesets: False
> >>> Finished checking the repository type.
> DEBUG:root:Running: git config --get reviewboard.url
> >>> HTTP GETting api/
> >>> HTTP GETting https://reviewboard.corp.ad.local/api/info/
> >>> Using the new web API
> DEBUG:root:Running: git merge-base origin/master refs/heads/working
> DEBUG:root:Running: git diff --no-color --full-index --no-ext-diff
> --ignore-submodules
> 08d87f9f8aa11cf60d2f6419fb9fe43d4d336967..refs/heads/working
> DEBUG:root:Running: git log --pretty=format:%s HEAD^..
> DEBUG:root:Running: git log --pretty=format:%s%n%n%b
> 08d87f9f8aa11cf60d2f6419fb9fe43d4d336967..
> >>> Attempting to create review request on 
> >>> g...@github.corp.ad.local:cm/transform.git
> for None
> >>> HTTP POSTing to https://reviewboard.corp.ad.local/api/review-requests/:
> {'repository': 'g...@github.corp.ad.local:cm/transform.git'}
> >>> Review request created
> >>> Attempting to set field 'target_groups' to 'PDDev' for review request
> '403'
> >>> HTTP PUTting to
> https://reviewboard.corp.ad.local/api/review-requests/403/draft/:
> {'target_groups': 'PDDev'}
> >>> Attempting to set field 'summary' to 'Test for PMDO-5475 -- ensure
> plan termination when metadata changed' for review request '403'
> >>> HTTP PUTting to
> https://reviewboard.corp.ad.local/api/review-requests/403/draft/:
> {'summary': 'Test for PMDO-5475 -- ensure plan termination when metadata
> changed'}
> >>> Attempting to set field 'description' to 'Test for PMDO-5475 -- ensure
> plan termination when metadata changed' for review request '403'
> >>> HTTP PUTting to
> https://reviewboard.corp.ad.local/api/review-requests/403/draft/:
> {'description': 'Test for PMDO-5475 -- ensure plan termination when
> metadata changed'}
> >>> Uploading diff, size: 8064
> >>> HTTP POSTing to
> https://reviewboard.corp.ad.local/api/review-requests/403/diffs/: {}
> >>> Got API Error 105 (HTTP code 400): One or more fields had errors
> >>> Error data: {u'fields': {u'path': [u"fatal: Not a git repository:
> 'None'\n"]}, u'stat': u'fail', u'err': {u'msg': u'One or more fields had
> errors', u'code': 105}}
>
> Error uploading diff
>
> The generated diff file was empty. This usually means no files were
> modified in this change.
>
> Try running with --output-diff and --debug for more information.
>
> Your review request still exists, but the diff is not attached.
>
>
> $ post-review --output-diff
> diff --git
> a/src/main/scala/com/globys/transform/physical/PhysicalEventTypeRegistry.scala
> b/src/main/scala/com/globys/transform/physical/PhysicalEventTypeRegistry.scala
> index
> 89401ab151cac0cc817e9acbe62280fbe57ccc98..f7c951d41f1a6d6bc2e1c594855c819ce272a61e
> 100644
> ---
> a/src/main/scala/com/globys/transform/physical/PhysicalEventTypeRegistry.scala
> +++
> b/src/main/scala/com/globys/transform/physical/PhysicalEventTypeRegistry.scala
> @@ -10,7 +10,7 @@ trait PhysicalEventTypeRegistry {
>    def getPhysicalEventType[T <: Event](eventTypeId: Short):
> PhysicalEventType[T]
>
>    /**
> -   * Throws IllegalArgumentException if event there is no event with the
> given name.
> +   * Throws IllegalArgumentException if event there is no eventType
> registered with the given name.
>     *
>     * @param name
>     * @return
> @@ -19,6 +19,12 @@ trait PhysicalEventTypeRegistry {
>
>    def registeredTypeNames: Seq[String]
>
> +  /**
> +   * Delete all the registered types.
> +   *
> +   */
> +  def unregisterAllPhysicalEventTypes(): Unit
> +
>    protected def getExistingEventType[T <: Event](eventType:
> EventType[T]): Option[PhysicalEventType[T]]
>
>    protected def createNewEventType[T <: Event](eventType: EventType[T]):
> PhysicalEventType[T]
> diff --git
> a/src/main/scala/com/globys/transform/physical/PhysicalEventTypeRegistryCarbonado.scala
> b/src/main/scala/com/globys/transform/physical/PhysicalEventTypeRegistryCarbonado.scala
> index
> c2c777af3701d6c329c64bc33b54fc8ef32f82bc..4c11276c68616451a2adf23ac5aa09971bd3be7a
> 100644
> ---
> a/src/main/scala/com/globys/transform/physical/PhysicalEventTypeRegistryCarbonado.scala
> +++
> b/src/main/scala/com/globys/transform/physical/PhysicalEventTypeRegistryCarbonado.scala
> @@ -35,6 +35,24 @@ class PhysicalEventTypeRegistryCarbonado(repository:
> Repository) extends Physica
>
>    def registeredTypeNames =
> eventTypeStorage.query().fetch().toList.asScala.map(_.getName)
>
> +  def unregisterAllPhysicalEventTypes() {
> +    val txn = repository.enterTransaction()
> +    try {
> +      eventTypeAttributeStorage.truncate()
> +      eventTypeStorage.truncate()
> +      txn.commit()
> +    }
> +    catch {
> +      case t => throw new IllegalStateException("Failed to
> unregsiterAllPhysicalEventTypes", t)
> +    }
> +    finally {
> +      txn.exit()
> +    }
> +    eventTypeForId = Map.empty
> +    eventTypeForName = Map.empty
> +  }
> +
> +
>    protected def getExistingEventType[T <: Event](eventType: EventType[T])
> = eventTypeForName.get(eventType.name) match {
>      case Some(pet) => Some(pet.asInstanceOf[PhysicalEventType[T]])
>      case None => loadByName(eventType.name).map(addToCache(_))
> diff --git
> a/src/test/scala/com/globys/transform/physical/local/ParallelTaskCoordinatorTest.scala
> b/src/test/scala/com/globys/transform/physical/local/ParallelTaskCoordinatorTest.scala
> index
> 4ec656fd00257b447249cfeb2f4bb61cc0802141..1b60b715d7a45597463939858d1df18e5dc15e68
> 100644
> ---
> a/src/test/scala/com/globys/transform/physical/local/ParallelTaskCoordinatorTest.scala
> +++
> b/src/test/scala/com/globys/transform/physical/local/ParallelTaskCoordinatorTest.scala
> @@ -1,6 +1,18 @@
>  package com.globys.transform.physical.local
>
> +import com.globys.integration.telcel.TelcelInfo
> +import com.globys.transform.GlobysUtil._
> +import com.globys.transform.main.{ProteusConfig, ProteusJob, UTConfig}
> +import org.joda.time.{Interval, DateTime}
>  import org.scalatest.FunSuite
> +import com.globys.transform.GlobysUtil._
> +import java.nio.file.{Path, Files, Paths}
> +import com.twitter.util.Config
> +import com.globys.transform.physical.RawFileType
> +import com.globys.transform.core.EventType
> +import org.scalatest.events.Event
> +import com.globys.transform.logical.reflective.ReflectiveEvent
> +import com.globys.integration.telcel.event.{Payment, VoiceCDR, Customer}
>
>  @org.junit.runner.RunWith(classOf[org.scalatest.junit.JUnitRunner])
>  class ParallelTaskCoordinatorTest extends FunSuite {
> @@ -18,4 +30,96 @@ class ParallelTaskCoordinatorTest extends FunSuite {
>      val result = ParallelTaskCoordinator.executeTasks(3, work)
>      assert(result.isInstanceOf[UnexpectedExit[_]])
>    }
> +  test("pdmo5475-memory") {
> +    val interval = new Interval(new DateTime(1980,10,15, 0,0), new
> DateTime(1980,10,18, 0,0))
> +    val plan = ProteusJob.readPlan("plans/migrationPlan.scala")
> +
> +    val config = new UTConfig(TelcelInfo)
> +    val registry = config.rawFileRegistryConfig.memoized
> +    val petr = config.physicalEventTypeRegistryConfig.memoized
> +
> +    def runPlanWith(files: List[(RawFileType, String)], types:
> List[EventType.Generic] = List.empty) {
> +      files.foreach { case (t, f) =>
> +        registry.registerFile(pathForResource(f).get, t,
> interval.getStart)
> +      }
> +      types.foreach { case t =>
> +        petr.getPhysicalEventType(t)
> +      }
> +      ProteusJob(config.localDataBaseBatchProcessorConfig, plan,
> interval).execute()
> +    }
> +
> +    // Run the plan
> +    runPlanWith(List(
> +      TelcelInfo.rechargeFile ->
> "telceldata/replenish_history_cs3_20120607_235959_ium.unl.reg9.gz",
> +      TelcelInfo.subscriberStatusFile ->
> "telceldata/subscriber_status_cs3_20120607_235959_ium_R9-part1.unl.gz"
> +    ),  List(
> +      ReflectiveEvent.eventType(classOf[Payment]),
> +      ReflectiveEvent.eventType(classOf[VoiceCDR])
> +    ))
> +
> +    // Blow away the metadata
> +    registry.unregisterAllFiles()
> +    petr.unregisterAllPhysicalEventTypes()
> +
> +    // Run the plan again (with a different files) and with different
> typeIds
> +    // This is supposed to fail -- the bug was that plan execution
> wouldn't terminate
> +    intercept[IllegalArgumentException] {
> +      runPlanWith(List(
> +        TelcelInfo.subscriberStatusFile
> ->"telceldata/subscriber_status_cs3_20120607_235959_ium_R9-part1.unl.gz",
> +        TelcelInfo.subscriberStatusFile
> ->"telceldata/subscriber_status_cs3_20120607_235959_ium_R9-part2.unl.gz",
> +        TelcelInfo.rechargeFile ->
> "telceldata/replenish_history_cs3_20120607_235959_ium.unl.reg9.gz"
> +      ), List(
> +        ReflectiveEvent.eventType(classOf[Customer])
> +      ))
> +    }
> +  }
> +
> +  test("pdmo5475-levelDB") {
> +    val interval = new Interval(new DateTime(1980,10,15, 0,0), new
> DateTime(1980,10,18, 0,0))
> +    val plan = ProteusJob.readPlan("plans/migrationPlan.scala")
> +
> +    val testDatadir = Paths.get("target/tmp/pdmo5475")
> +    recursivelyDelete(testDatadir)
> +    Files.createDirectories(testDatadir)
> +
> +    def runPlanWith(files: List[(RawFileType, String)], types:
> List[EventType.Generic] = List.empty) {
> +        val config = new ProteusConfig("telcel") {
> +          fileInfo = TelcelInfo
> +          datadir = testDatadir
> +        }
> +
> +      val registry = config.rawFileRegistryConfig.memoized
> +      files.foreach { case (t, f) =>
> +        registry.registerFile(pathForResource(f).get, t,
> interval.getStart)
> +      }
> +
> +      // Create some random types to mess with the indexes
> +      val petr = config.physicalEventTypeRegistryConfig.memoized
> +      types.foreach { case t => petr.getPhysicalEventType(t) }
> +
> +      ProteusJob(config.localDataBaseBatchProcessorConfig, plan,
> interval).execute()
> +    }
> +
> +    // Run the plan
> +    runPlanWith(List(
> +      TelcelInfo.rechargeFile ->
> "telceldata/replenish_history_cs3_20120607_235959_ium.unl.reg9.gz",
> +      TelcelInfo.subscriberStatusFile ->
> "telceldata/subscriber_status_cs3_20120607_235959_ium_R9-part1.unl.gz"
> +    ),  List(
> +      ReflectiveEvent.eventType(classOf[Payment]),
> +      ReflectiveEvent.eventType(classOf[VoiceCDR])
> +    ))
> +
> +    // Blow away the metadata
> +    recursivelyDelete(testDatadir.resolve("metadata"))
> +
> +    // Run the plan again (with a different files) and with different
> typeIds
> +    // Kind of disappointing this works given the metadata and event
> store are out of sync
> +    runPlanWith(List(
> +      TelcelInfo.subscriberStatusFile
> ->"telceldata/subscriber_status_cs3_20120607_235959_ium_R9-part1.unl.gz",
> +      TelcelInfo.subscriberStatusFile
> ->"telceldata/subscriber_status_cs3_20120607_235959_ium_R9-part2.unl.gz",
> +      TelcelInfo.rechargeFile ->
> "telceldata/replenish_history_cs3_20120607_235959_ium.unl.reg9.gz"
> +    ), List(
> +      ReflectiveEvent.eventType(classOf[Customer])
> +    ))
> +  }
>  }
>
>  --
> Want to help the Review Board project? Donate today at
> http://www.reviewboard.org/donate/
> Happy user? Let us know at http://www.reviewboard.org/users/
> -~----------~----~----~----~------~----~------~--~---
> To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~----------~----~----~----~------~----~------~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Reply via email to