Skip to content
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

Add extra options to session class #236

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

philipwhiuk
Copy link
Contributor

@philipwhiuk philipwhiuk commented Aug 16, 2019

We have long had some internal settings that I feel like upstream will benefit from (also it makes my life easier).

AllowPosDup

PossDupFlag is set when resending messages. Currently it's automatically removed by QFJ.
This PR makes two changes:

  • It exposes a method 'sendNoChange' which never removes these settings. This is useful when resending messages from a cache or from an application store behind QFJ.
  • It exposes a session setting 'AllowPosDup' that means we never remove the flag. This has been useful on occasion - primarily when a QFJ application is acting as purely a pass-through/monitoring hop.

Fix41ResendRequestAsFix42

  • Some sessions claim to be FIX.4.1 but actually send FIX.4.2 format ResendRequests. This is useful for that. Quite common in less formalised trading networks where quite frankly the begin string is often just misleading.

Copy link
Member

@chrjohn chrjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @philipwhiuk ,

thanks for the PR. I have some questions:

Fix41ResendRequestAsFix42
Is it sensible to change this behaviour only for the receiving side? Don't you want to send ResendRequest for FIX4.1 with infinity conveyed as 0 also?
In general, how do do you feel about renaming this option? Maybe something along the lines of ResendUsesZeroAsInfinity or something like that? On FIX4.2+ this option would of course be enabled implicitly.
I also do not know if it makes sense to make this a FIX4.1 only feature. The same could apply to FIX4.0.

Still thinking about the PossDup stuff but sounds useful.

Cheers,
Chris.

@philipwhiuk
Copy link
Contributor Author

philipwhiuk commented Aug 20, 2019

I'll have to work out how to handle it if we rename them. We can probably deprecate and map the old values in our code before passing them to QFJ. I'm open to renaming it and making it more general.

In terms of sending that does make sense. I'll see if we have any active usage of it these days (it's been internal for way way too long) and see whether we can try to see whether it was intentionally only done on one side.

I'll try to write some more tests for both pieces before we merge this in :)

@esanchezros
Copy link
Contributor

Hi @chrjohn,

What needs doing on this PR? I'm happy to help moving it forward

Thanks

@philipwhiuk
Copy link
Contributor Author

Hi @esanchezros. I'll update this tomorrow with some tests and the changes from Chris and then hopefully it can be merged :)

@esanchezros
Copy link
Contributor

Hi @philipwhiuk,
I thought this PR was abandoned and created a new one just for allowing PossDupFlag in send #395
If you and @chrjohn are okay with it perhaps it can be reviewed/merged and you could create a separate one for the other feature?
Thanks
Ed

@chrjohn
Copy link
Member

chrjohn commented May 21, 2021

Hi @philipwhiuk @esanchezros

personally I would prefer to have each feature in a single PR, so if it is OK with you (@philipwhiuk) then I would be happy if we could proceed with #395 for the PossDup thing and leave this PR for the FIX4.1/4.2 Resend option.

What do you think?
Thanks,
Chris.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants