New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue 1125 - BodyParam were broken with default values #1129

Merged
merged 2 commits into from Jun 10, 2015

Conversation

Projects
None yet
3 participants
@Geod24
Contributor

Geod24 commented Jun 10, 2015

I have no idea how it used to compile...

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jun 10, 2015

Member

Looks good. New DUB release is needed to get rid of that error.

Member

s-ludwig commented Jun 10, 2015

Looks good. New DUB release is needed to get rid of that error.

s-ludwig added a commit that referenced this pull request Jun 10, 2015

Merge pull request #1129 from Geod24/fix-1125
Fix issue 1125 - BodyParam were broken with default values

@s-ludwig s-ludwig merged commit 2c3edfc into vibe-d:master Jun 10, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@Geod24

This comment has been minimized.

Show comment
Hide comment
@Geod24

Geod24 Jun 10, 2015

Contributor

Thanks !

Looks good. New DUB release is needed to get rid of that error.

Any ETA ? Also any plan for a Vibe.d release ?

Contributor

Geod24 commented Jun 10, 2015

Thanks !

Looks good. New DUB release is needed to get rid of that error.

Any ETA ? Also any plan for a Vibe.d release ?

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jun 10, 2015

Member

I would like to wait for dlang/dub#582 to get reviewed/merged, but if that doesn't happen in the near term, I'd just start the release process based on the current status (there are mostly only bug fixes since 0.9.23).

The next vibe.d release should again happen in sync with DMD 2.068. The only thing I still want to review/finish is the start of the transition to using scope for HTTP request handlers, the next big changes, such as the HTTP/2 support will be something for the 0.7.25 release (maybe it makes sense to call it 0.8.0 for a better marketing effect).

Member

s-ludwig commented Jun 10, 2015

I would like to wait for dlang/dub#582 to get reviewed/merged, but if that doesn't happen in the near term, I'd just start the release process based on the current status (there are mostly only bug fixes since 0.9.23).

The next vibe.d release should again happen in sync with DMD 2.068. The only thing I still want to review/finish is the start of the transition to using scope for HTTP request handlers, the next big changes, such as the HTTP/2 support will be something for the 0.7.25 release (maybe it makes sense to call it 0.8.0 for a better marketing effect).

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Jun 10, 2015

Contributor

The only thing I still want to review/finish is the start of the transition to using scope for HTTP request handlers, the next big changes, such as the HTTP/2 support will be something for the 0.7.25 release (maybe it makes sense to call it 0.8.0 for a better marketing effect).

Again many unmerged changes on my fork, I tested everything 1 million times, for leaks as well. Seems good now.

A new Task debugging feature is there as well on

etcimon@d8006c2#diff-ab96042d1071a2f6d25b2d780277a06bR37

See screenshot: http://i.imgur.com/vpVcDnN.jpg

Contributor

etcimon commented Jun 10, 2015

The only thing I still want to review/finish is the start of the transition to using scope for HTTP request handlers, the next big changes, such as the HTTP/2 support will be something for the 0.7.25 release (maybe it makes sense to call it 0.8.0 for a better marketing effect).

Again many unmerged changes on my fork, I tested everything 1 million times, for leaks as well. Seems good now.

A new Task debugging feature is there as well on

etcimon@d8006c2#diff-ab96042d1071a2f6d25b2d780277a06bR37

See screenshot: http://i.imgur.com/vpVcDnN.jpg

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Jun 10, 2015

Contributor

As a side note, I wrote this at first as a tool to find leaks, but I found it to be a nice solution for many other things because the link time in Windows is absolutely horrible in --build=debug and I had no symbols in --build=plain, so the errors get thrown with a nice call stack that I can push/pop elements automatically with mixin(Trace);:

etcimon@1951cf2#diff-5a483448dcc1a27cb132021d682e1d1bR279

I think the debugger could gradually become a sort of process manager with debug information, I intend on making a nice GUI javascript framework for it.

Contributor

etcimon commented Jun 10, 2015

As a side note, I wrote this at first as a tool to find leaks, but I found it to be a nice solution for many other things because the link time in Windows is absolutely horrible in --build=debug and I had no symbols in --build=plain, so the errors get thrown with a nice call stack that I can push/pop elements automatically with mixin(Trace);:

etcimon@1951cf2#diff-5a483448dcc1a27cb132021d682e1d1bR279

I think the debugger could gradually become a sort of process manager with debug information, I intend on making a nice GUI javascript framework for it.

@Geod24

This comment has been minimized.

Show comment
Hide comment
@Geod24

Geod24 Jun 10, 2015

Contributor

@s-ludwig : A release of dub soonish would be awesome, because until then, vibe.d master will be red, and that kinda defeat the purpose of CI :)

Contributor

Geod24 commented Jun 10, 2015

@s-ludwig : A release of dub soonish would be awesome, because until then, vibe.d master will be red, and that kinda defeat the purpose of CI :)

@Geod24 Geod24 deleted the Geod24:fix-1125 branch Jun 10, 2015

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jun 11, 2015

Member

We can add a quick workaround to travis.yml in the meantime (first do dub upgrade --missing-only and then use --nodeps).

@etcimon I actually have a graphical debugger in the works (didn't have time to work on it since a while though). We should definitely try to at least avoid duplicate work here.

redebug

Member

s-ludwig commented Jun 11, 2015

We can add a quick workaround to travis.yml in the meantime (first do dub upgrade --missing-only and then use --nodeps).

@etcimon I actually have a graphical debugger in the works (didn't have time to work on it since a while though). We should definitely try to at least avoid duplicate work here.

redebug

s-ludwig added a commit that referenced this pull request Jun 11, 2015

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jun 11, 2015

Member

Turns out that the package description of the new test was actually wrong. Should hopefully go green now.

Member

s-ludwig commented Jun 11, 2015

Turns out that the package description of the new test was actually wrong. Should hopefully go green now.

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Jun 11, 2015

Contributor

@etcimon I actually have a graphical debugger in the works (didn't have time to work on it since a while though). We should definitely try to at least avoid duplicate work here.

It shouldn't be duplicate work, I didn't know you did that btw.

My problem is, I currently find it very hard to write and debug new pages because of all the side effects involved in a complex web application. I get 16mb logs for a few requests, so I'm working on a fiber filtering logging feature. I also don't really have any way to know if fibers have hung or where they hung. It's also a pain to build with symbols all the time the windows linker starts over because of it.

So I actually want to make it a JSON API sort of "task process manager", I have the memory usage per task now, and I can "interrupt" hung tasks and see where they hung even on release builds.

The most important feature for me is the "Recording with filters" feature I'm working on right now. It's a little tiring to have to use log files for each task indiscriminately to examine one little thing

e.g. When you request:
http://localhost:8080/record_once/trace,headers,json_code,category1,category2/the/path/to/debug/goes/here

The page will simply wait for a request to http://localhost:8080/the/path/to/debug/goes/here to complete, and will return when it has captured whatever was declared using the following syntax:
mixin(IfRecording!("headers", "req.headers")) or mixin(IfRecording!("json_code", "mystruct.serializeToJsonString()")), for the task that has the specific breadcrumbs that match this request.

If trace was specified, the mixin(Trace) will be part of the report. I need it to work on release builds as well, it would have to be a debug-for-production tool (very important).

I think the two tools can actually be merged in some way since this will be an API.

Contributor

etcimon commented Jun 11, 2015

@etcimon I actually have a graphical debugger in the works (didn't have time to work on it since a while though). We should definitely try to at least avoid duplicate work here.

It shouldn't be duplicate work, I didn't know you did that btw.

My problem is, I currently find it very hard to write and debug new pages because of all the side effects involved in a complex web application. I get 16mb logs for a few requests, so I'm working on a fiber filtering logging feature. I also don't really have any way to know if fibers have hung or where they hung. It's also a pain to build with symbols all the time the windows linker starts over because of it.

So I actually want to make it a JSON API sort of "task process manager", I have the memory usage per task now, and I can "interrupt" hung tasks and see where they hung even on release builds.

The most important feature for me is the "Recording with filters" feature I'm working on right now. It's a little tiring to have to use log files for each task indiscriminately to examine one little thing

e.g. When you request:
http://localhost:8080/record_once/trace,headers,json_code,category1,category2/the/path/to/debug/goes/here

The page will simply wait for a request to http://localhost:8080/the/path/to/debug/goes/here to complete, and will return when it has captured whatever was declared using the following syntax:
mixin(IfRecording!("headers", "req.headers")) or mixin(IfRecording!("json_code", "mystruct.serializeToJsonString()")), for the task that has the specific breadcrumbs that match this request.

If trace was specified, the mixin(Trace) will be part of the report. I need it to work on release builds as well, it would have to be a debug-for-production tool (very important).

I think the two tools can actually be merged in some way since this will be an API.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jun 12, 2015

Member

The API level (and below) is where I expected the duplication to happen. What I have is a small library that hooks into the vibe.d application using the Logger interface, as well as setTaskEventCallback. It uses a custom stack tracer for high performace tracing and then uses a binary protocol to talk to the GUI application for efficiency reasons. On top of that it provides some functions to trigger manual debug events or record certain values.

If that would be interesting for you, I could make this part of the project public. It's just a smallish 300 loc library plus the stack trace code.

Member

s-ludwig commented Jun 12, 2015

The API level (and below) is where I expected the duplication to happen. What I have is a small library that hooks into the vibe.d application using the Logger interface, as well as setTaskEventCallback. It uses a custom stack tracer for high performace tracing and then uses a binary protocol to talk to the GUI application for efficiency reasons. On top of that it provides some functions to trigger manual debug events or record certain values.

If that would be interesting for you, I could make this part of the project public. It's just a smallish 300 loc library plus the stack trace code.

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