Checking build-version vs. runtime version #577

Closed
staabm opened this Issue Jan 25, 2013 · 11 comments

Comments

Projects
None yet
4 participants
Member

staabm commented Jan 25, 2013

Hi,

I recently had a problem with differences between the dev environment and a test server.

After some investigation I found the problem. The reason was, that I had compiled/build the propel project on DEV with a GIT version of propel, commited it into our SVN, updated the test-server with this changes and got some crazy errors which were not reproducable on DEV.

The actual problem was reported as a unknown method "postHydrate", which atm only exists in propel GIT, but not in the lastest release.
Since this error was very misleading, I would propose a check while Propel-Initialization which compares the version of propel which was used at build time and the one which is used at runtime.

would you welcome a PR for that or do you think this is considered a new feature and only be applied for Propel2?

Thanks,
Markus

Member

jaugustin commented Jan 25, 2013

@staabm if I understand you problem is you build propel class with version 1.6.X and run them with propel 1.6.Y

I think the best practice for that is to never version generated classes but to (re)build them on the server, that way you always has the right classes ;)

Member

staabm commented Jan 25, 2013

You are absolutely right, but don't you think a exception in this case will also help the dev so he has a hint what happend. I hate wasting time for errors which are not obvious.

you could also think about another use-case:
when shipping a new release for a software which is based on a newer version of propel than the previous release, such an exception would help when you deploy the new release on a server which propels-runtime wasn't updated in the first place (because it was forgotten).

sure we could adjust our workflow and deployment, but you also know that things happen, even if your workflow was designed to handled those issues...

I didn't had a look in the source how we could implement this little feature, but if you accept it, I would be willing to provide an PR (either for propel1, propel2 or both)

Member

jaugustin commented Jan 25, 2013

I understand but how to test it ? you want to generate a BUILD_VERSION file ? and check it when ?

Member

staabm commented Jan 25, 2013

After digging a little further, I found that propel already generates the generator version into the *-conf.php, so we need only to compare this config value with the one of the runtime.

Member

staabm commented Jan 25, 2013

something along the lines

        if (self::$configuration['generator_version'] != self::VERSION) {
            throw new PropelException("Version mismatch: The generated model was build using propel '". self::$configuration['generator_version'] ."' while the current runtime is at '". self::VERSION ."'");
        }

in the initilize() method should do...

Owner

marcj commented Jan 25, 2013

+1

Owner

willdurand commented Jan 25, 2013

Well, an exception is a bit too much. I guess using trigger_error() would
be enough.

William Durand | http://www.williamdurand.fr

On Fri, Jan 25, 2013 at 5:09 PM, MArc J. Schmidt
notifications@github.comwrote:

+1


Reply to this email directly or view it on GitHubhttps://github.com/propelorm/Propel/issues/577#issuecomment-12707206.

Member

staabm commented Jan 25, 2013

Ok, i can do that. Just for propel1, propel2 or both?

Owner

willdurand commented Jan 25, 2013

both?

William Durand | http://www.williamdurand.fr

On Fri, Jan 25, 2013 at 6:25 PM, Markus Staab notifications@github.comwrote:

Ok, i can do that. Just for propel1, propel2 or both?


Reply to this email directly or view it on GitHubhttps://github.com/propelorm/Propel/issues/577#issuecomment-12711111.

Owner

marcj commented Jan 25, 2013

both!

Member

staabm commented Jan 26, 2013

added a PR for propel1. please review. when you like the way I did it, I will add the same for propel2.

staabm closed this Apr 4, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment