Christian,

Actually I'm sure going back to the old raw-file URL will work -- we did 
some tests -- but like the GitHub service, the latest version of the 
self-hosted GitHub also drops the v2 api and requires the username/password 
pair.

The part that confuses me, is that a simple diff works, but a more 
complicated diff doesn't. What's different?

The longer diff causes the post-review or the review board API to loose 
track of the git repository -- the debug output for the simple diff is 
quite happy and knows what the repository is, but the debug from the 
complex diff says ">>> 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}}" and this is confirmed in the 
review server logs:
2012-10-05 08:54:11,419 - ERROR - Error uploading new diff: fatal: Not a 
git repository: 'None'
Traceback (most recent call last):
  File 
"/usr/lib/python2.6/site-packages/ReviewBoard-1.6.12-py2.6.egg/reviewboard/webapi/resources.py",
 
line 1509, in create
    request.FILES.get('parent_diff_path'))
  File 
"/usr/lib/python2.6/site-packages/ReviewBoard-1.6.12-py2.6.egg/reviewboard/reviews/forms.py",
 
line 296, in create
    history)
  File 
"/usr/lib/python2.6/site-packages/ReviewBoard-1.6.12-py2.6.egg/reviewboard/diffviewer/forms.py",
 
line 84, in create
    diff_file, basedir, check_existance=(not parent_diff_file)))
  File 
"/usr/lib/python2.6/site-packages/ReviewBoard-1.6.12-py2.6.egg/reviewboard/diffviewer/forms.py",
 
line 170, in _process_files
    not self.repository.get_file_exists(filename, revision))):
  File 
"/usr/lib/python2.6/site-packages/ReviewBoard-1.6.12-py2.6.egg/reviewboard/scmtools/models.py",
 
line 169, in get_file_exists
    return self.get_scmtool().file_exists(path, revision)
  File 
"/usr/lib/python2.6/site-packages/ReviewBoard-1.6.12-py2.6.egg/reviewboard/scmtools/git.py",
 
line 82, in file_exists
    return self.client.get_file_exists(path, revision)
  File 
"/usr/lib/python2.6/site-packages/ReviewBoard-1.6.12-py2.6.egg/reviewboard/scmtools/git.py",
 
line 398, in get_file_exists
    contents = self._cat_file(path, revision, "-t")
  File 
"/usr/lib/python2.6/site-packages/ReviewBoard-1.6.12-py2.6.egg/reviewboard/scmtools/git.py",
 
line 440, in _cat_file
    raise SCMError(errmsg)
SCMError: fatal: Not a git repository: 'None'




On Friday, October 5, 2012 1:15:14 PM UTC-7, Christian Hammond wrote:
>
> 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 - chi...@chipx86.com <javascript:>
> Review Board - http://www.reviewboard.org
> VMware, Inc. - http://www.vmware.com
>
>
> On Fri, Oct 5, 2012 at 8:59 AM, Joe <jfri...@globys.com <javascript:>>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...@googlegroups.com <javascript:>
>> 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