Acceptor Dynamic Session Templates #607
Conversation
There was a problem hiding this comment.
Ok, first off, this is going to have some conflicts with #609. That's not a big PR, but it does make a few changes to Session and SessionFactory. I hope to merge #609 today.
I'm not crazy about the acceptor-specific additions to Session. Can we think of any other approach? Making a DynamicSession subclass seems possible off the top of my head, but also could be horrible.
Don't go deep into any big changes yet. I also want to talk to @mgatny and see what ideas he has.
| /// Calculates unique filename prefix from SessionID. | ||
| /// Handles wildcards in SessionID fields | ||
| /// </summary> | ||
| /// <returns>Filename prefix unique for SessioID</returns> | ||
| public static string Prefix(SessionID sessionID) |
There was a problem hiding this comment.
This is mostly (or all) the same as FileStore.Prefix(). That was fine when they were small, but now I think this common code should be unified. Perhaps a static class & function would be appropriate?
There was a problem hiding this comment.
I can do - should I put the new calls in "Util" subfolder of the solution?
There was a problem hiding this comment.
That'll work, or it could go in the same dir as FileLog/FileStore. Doesn't really matter to me.
| public Session( | ||
| IApplication app, IMessageStoreFactory storeFactory, SessionID sessID, DataDictionaryProvider dataDictProvider, | ||
| SessionSchedule sessionSchedule, int heartBtInt, ILogFactory logFactory, IMessageFactory msgFactory, string senderDefaultApplVerID, | ||
| IAcceptor acceptor, QuickFix.Dictionary settings) |
There was a problem hiding this comment.
I'm not terribly comfortable seeing _acceptor here given how this class isn't really acceptor-specific, but I don't have a better idea. Not totally sure about settings also.
There was a problem hiding this comment.
Both _acceptor and _settings are required to create the new "real" session when connecting to a template session. However, as noted in the beginning, it may not be the best solution and might be replaced with adding a SessionTemplate class. What, in turn, then seemingly requires more changes to the logic of accepting connections and looking up existing sessions...
There was a problem hiding this comment.
Yeah, I get that. I just don't like the resulting architecture. I'll think about it, see if any brainstorms come to me...
| { | ||
| throw new ConfigError(BEGINSTRING + " (" + beginString + ") must be FIX.4.0 to FIX.4.4 or FIXT.1.1"); | ||
| throw new ConfigError($"{BEGINSTRING} ({ beginString }) must be FIX.4.0 to FIX.4.4 or FIXT.1.1 or wildcard '{Values.WILDCARD_VALUE}' (only for acceptor)"); |
There was a problem hiding this comment.
How can we wildcard the begin string? An acceptor can only service one FIX version.
There was a problem hiding this comment.
I got "inspired" by simpleacc.cfg, where one acceptor has several [SESSION] with different BeginString. Now, at second glance, I realize that DataDictionary needs to also be dynamically adjusted to BeginString, and that would probably be complicated to solve, but not really worth it... So I would agree to remove wildcard support for BeginString parameter.
Fine, I will merge #609 into this PR as well (after you).
I did start with a new DynamicSession class but then realized that it would require more complicated changes than "simply" replacing LookupSession with LookupOrCreateSession (now called GetSession), so decided for a quicker solution.
OK, I would wait for your further advice before making the suggested changes. Thank you! |
|
FYI, #609 is in master now, so at your leisure you can upmerge or rebase. |
|
I understood that I should wait for decision on architecture - is the
approach with passing acceptor into a session good enough? Or should it be
changed, probably by adding a class for "template" session?
…On Tue, 16 Jun 2020, 17:54 Grant Birchmeier, ***@***.***> wrote:
FYI, #609 <#609> is in master
now, so at your leisure you can upmerge or rebase.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#607 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKABHZT5FA7TSPBMOK4MIE3RW6IU5ANCNFSM4NR74BKA>
.
|
|
Haven't had time to think about reworking the impl yet, but for now maybe just resolve the conflicts. |
|
OK, will resolve |
Catching up with the original master
pvkv1799
force-pushed
the
wildcard_session
branch
from
2d4c15f
to
a63ffdc
Compare
|
Conflicts resolved, and some of the suggested fixes made (marked them resolved). The rest will do after decision on the architecture. |
|
@gbirchmeier Apologies - have you had a chance to think about the architecture? I could do the changes if you have a better solution in mind. |
…is not recognized as an internal or external command, operable program or batch file."
Bumps [NUnit3TestAdapter](https://github.com/nunit/nunit3-vs-adapter) from 3.16.1 to 3.17.0. - [Release notes](https://github.com/nunit/nunit3-vs-adapter/releases) - [Commits](nunit/nunit3-vs-adapter@V3.16.1...V3.17) Signed-off-by: dependabot-preview[bot] <[email protected]>
forgot to do this on 1.10.0 release, oops
2.7 is only on the VS2019 env, but appveyor is not letting me change to that env.
…ration string as a parameter instead of the hard-coded constant
Added wildcards validation into SessionSettings; Moved WILDCARD_VALUE from SessionSettings to Values; Added Unit Tests for wildcards for the recently changed classes
…incoming connection
pvkv1799
force-pushed
the
wildcard_session
branch
from
76b4b61
to
2f54fe5
Compare
|
|
Enables "*" wildcard in Acceptor session configuration (for BeginString, SenderCompID, SenderSubID, SenderLocationID, TargetCompID, TargetSubID, and TargetLocationID).
Such session templates can be defined either at start-up or as an ad-hoc operation, just like "normal" sessions.
When processing a new incoming connection, if incoming SessionID is not equal to any of the created sessions, it is then matched considering wildcards. If such a match is found, the new session is created with the same settings as the template one, just changing wildcards to the actual values from incoming SessionID.