search

minodudd / TeamCity-plugins

Apache License 2.0
10 stars 8 forks source link

TeamCity 9 RC #1

Open chrisdrobison opened 9 years ago

chrisdrobison commented 9 years ago

I've tried using your plugin on TeamCity 9 RC 1 and it doesn't seem to work. It's not adding the new parameter.

chrisdrobison commented 9 years ago

Any progress on this? Can I help in any way?

minodudd commented 9 years ago

To be honest, I haven't got around to it and don't see it happening in the next week or two. I'll try to run it on TC 9 towards the end of the month and see what's causing the issue.

dnak commented 9 years ago

Hi, I experience this issue while looking to migrate to TeamCity. I plugged the problem on this fork, but I haven't thoroughly tested. It has only been used in a proof of concept environment.

It appeared that the build.getStartDate(); method from the TC is attempting to return a null, which in turn throws an exception. I simply used the current DateTime instead, but I don't know enough about the ramifications of that.

minodudd commented 9 years ago

In that case, the problem is that the parameter is no longer based on the build's start time, but instead on whenever the plugin was executed.

I'd use it as a backup, meaning:

// get the start date
Date buildStartDate = (build.getStartDate() != null ? build.getStartDate() : new Date());

This way we either use the proper start time, or if unavailable, use this approximate time.

Feel free to send me a pull request :)

minodudd commented 9 years ago

I've built the plugin with the change above but I have no TC9 system to test it on (nor the time to start one). @chrisdrobison, would you be interested in testing it on your system? I'm hosting it here for a few days.

dnak commented 9 years ago

Sorry, I left out some details. It appears that the build.getStartDate() on the TC api has an @NotNull annotation. This constraint is being violated (meaning TC is attempting to return null) and this is actually the source of the exception. So this appears to be a TeamCity issue. I haven't followed up with TC.

So, yeah, the fix I've got there is incorrect. We could do something similar to what you have there but it would still require some error handling around the build.getStartDate() call.

I don't have access to our logs right now but I can try to dig them up over the next several days and share the stack trace.

minodudd commented 9 years ago

Got it. I'll replace the implicit if with proper try/catch and upload another version of the plugin for you guys to test.

minodudd commented 9 years ago

Done, it's available on the same URL as before.

chrisdrobison commented 9 years ago

Yes, I will run it and see what happens.

chrisdrobison commented 9 years ago

Looks like it's working. I manually changed the time on the system I'm using multiple times and the new builds picked it up immediately.

minodudd commented 9 years ago

Cool, I'll update the plugin on my blog as well. Thanks for all your help :)