-
-
Notifications
You must be signed in to change notification settings - Fork 22
remove s3 bucket polling #1049
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?
remove s3 bucket polling #1049
Conversation
…_since.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
servicex_app/servicex_app/resources/internal/get_dataset_files_since.py
Outdated
Show resolved
Hide resolved
Looks good to me. I guess by default the index will be a B-tree and will be useful for the > operator? |
Another question, for broader discussion - is it finally time for us to introduce a "capabilities" endpoint? In this case we could use it so the client could verify that this new endpoint is available and switch behavior appropriately. (Mostly I am starting to worry about different sites running different versions of the server software but all users picking up the latest versions of the clients.) |
Yeah, any index will be B-tree by default and I think it's good practice to over-index anything that might be used to filter values. |
Can these capabilities be reduced down to version numbers? If so, you could set the client version as a HTTP header, version the endpoints, and create a middleware to fail any request without a sufficient client version for the endpoint. I spent some time hacking with Claude and think an interface like this should be possible:
|
@ponyisi @BenGalewsky I made major changes to this PR, please take a look. I noticed that files were available in the |
@ponyisi @BenGalewsky added the feedback from Friday, including the cutoff/begin_at value to the endpoint. |
|
||
args = parser.parse_args() | ||
|
||
transform_result_query = TransformationResult.query.filter_by(request_id=request_id) |
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.
do we have an index on request_id
?
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.
We do not. I will add one.
Please contribute to the quality of our codebase with unit tests for this new endpoint |
Added in preparation for front end changes per #1006