-
Notifications
You must be signed in to change notification settings - Fork 20
add RSS feed endpoint for querying AQ message views #1512
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
base: develop
Are you sure you want to change the base?
Conversation
MikeNeilson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I like it. I do want to make sure that if at all practical we're exposing more standard "queue/topic" semantics.
While this will always be read only, WM staff and core developers will be interacting with the same sources of data and we should make sure things map, preferably into the future and not the past.
Will very likely be making good use of SNS and SQS for queues and topics (of which this is technically the later regardless of Oracle naming).
I'm also currently inclined towards a "queue/topic per concept" vs per-office per-concept. The latter may be an impratical amount of things to deal with when the applications can easily choose to filter and let whatever broker deal.
|
|
||
| private Result<?> retrieveMessages(int offset, int pageSize, Instant since, String office, AqTable name) { | ||
| Timestamp sinceTimestamp = since == null ? null : Timestamp.from(since); | ||
| Table<?> t = table(name("CWMS_20", "AQ$" + office + "_" + name.name() + "_TABLE")).as("t"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting trick, taking advantage of the queue persistence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah makes it so that we don't have to pull in all the aqapi and jms dependencies and keeps us from being stateful
cwms-data-api/src/test/resources/cwms/cda/data/dto/rss/ts_stored.xml
Outdated
Show resolved
Hide resolved
MikeNeilson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your new tests seem to be breaking, but seems like a reasonable start.
Alternate considerations:
sincefield.