ats_fastcgi experimental plugin #3749
Conversation
|
The RAT complaints are on trivial files in There's one complaint that needs a closer look, by someone more knowledgeable then me: |
|
My preference would be that you add the license blurb where possible. I.e. if a config file can not handle comments, then you can add that particular file to the RAT exclusion, but in general, it's just a lot better to minimize the exclusions when at all possible. |
|
RAT & clang passing, but now autest complains, at a glance on seemingly unrelated stuff |
|
Some more notes:
Samples of .NET handling requests: |
|
(resolved small conflict in rat excludes that arose by a commit on master) |
|
|
||
| namespace ats_plugin | ||
| { | ||
| // A macro to disallow the copy constructor and operator= functions |
There was a problem hiding this comment.
Would this better as either an inherited class or using something like
TypeName(const TypeName&) = delete;
| void | ||
| ComputeStartTime() | ||
| { | ||
| this->start_time_ = static_cast<std::size_t>( |
There was a problem hiding this comment.
You could declare that as std::chrono::high_resolution_clock::rep to avoid the cast and make other operations easier later. Also I have had some problems with time_since_epoch for std::chrono::high_resolution_clock on some compilers. I ended up using std::chrono::system_clock and static_assert to verify the resolution was sufficiently fine.
| } | ||
|
|
||
| void | ||
| profileLength() |
There was a problem hiding this comment.
Perhaps better as printProfileLength() since it does the printing?
| Headers &h = transaction.getClientRequest().getHeaders(); | ||
| if (h.isInitialized()) { | ||
| atscppapi::header_field_iterator it = h.begin(); | ||
| for (it = h.begin(); it != h.end(); ++it) { |
There was a problem hiding this comment.
Better to do a range-based for loops, or clang-tidy will change it
| * limitations under the License. | ||
| */ | ||
|
|
||
| #ifndef REQUEST_QUEUE_H |
There was a problem hiding this comment.
We switch to use pragma once
|
@bryancall @SolidWallOfCode thanks for the feedback. |
|
So, I set up this plugin on a development machine based on the master branch, for addressing the comments in here. Previously, on 7.x it worked, but on master the plugin hangs after the intercept is hooked. This requires some digging into first, before I can go on and address comments. |
oschaaf
force-pushed
the
oschaaf-ats-fastcgi
branch
from
c2b99df to
bae6692
Compare
This change unbreaks using cppapi ServerIntercepts (Ran into a hanging FastCGI plugin while trying address comments in #3749)
|
Once #3927 lands, I plan to rebase this and proceed to addressing comments |
oschaaf
force-pushed
the
oschaaf-ats-fastcgi
branch
from
bae6692 to
129dcb5
Compare
|
I think I addressed the comments in here, PTAL? |
|
@bryancall I addressed your comment, I think, WDYT? |
|
@bryancall ping :-) |
|
@oschaaf looks good, can you please squash the commits |
|
@bryancall Sure, I will squash them |
oschaaf
force-pushed
the
oschaaf-ats-fastcgi
branch
from
c1030af to
bf676fd
Compare
As presented at the summit in Cork. See README.md for more information.
oschaaf
force-pushed
the
oschaaf-ats-fastcgi
branch
from
bf676fd to
51c40a2
Compare
|
@bryancall Rebased onto the head of master, squashed & force pushed. |
|
Cherry picked to 8.1.0 |
4 participants
As presented at the summit in Cork.
See
README.mdfor more information.