Support update #126
Open
Support update #126
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
requires that qlever also accepts the access token from the authorization header file
hannahbast
requested changes
Jan 24, 2025
hannahbast
requested changes
Feb 7, 2025
There was a problem hiding this comment.
1-1 with Julian, this looks great and works. A few suggestions for improvement:
- Remove the extra space before the "Automatically add names ..."
- Add button for "Clear delta triples" and "Clear cache complete"
- Add some more information to the "Update successful" message
NOTE: There already is a field for the access token (called API token) in the backend config as well as buttons for clear cache etc.
The whole warmup functionality is broken quite much (see #130). Fixing that that is out of scope, so I won't add a corresponding |
hannahbast
requested changes
Feb 13, 2025
There was a problem hiding this comment.
Thorough 1-1 with Julian, this is great and works fine now. A few more glitches:
- Functionality of the new buttons: when successful after the red x, the spinning wheel icon disappears completely
- When buttons are displayed the whole column moves to the left; more padding
- Number of inserted triples is 0 although it isn't (maybe same for delete triples)
- Show backend command buttons (Clear Delta Triples, Clear Cache (Complete) iff Access Token is not empty. Fixed that the buttons would not be shown if the field was not empty on load. - Backend command button icons are set correctly when transitioning between states (running, failed, succeeded). The buttons now also return to the green color after a successful execution. - Columns of Basic settings table in Backend Information changed widths when the backend command buttons where shown/hidden. Fixed by having them beneath each other. - Backend command buttons have some padding. - If the operation type was not detected this could also cause indefinite operation execution (although this seems to not be the only cause). Display an error in this case and prompt to report it.
|
6b0b2e3 contains an extensive description of the changes and fixes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Could also be made to work with the current setup of having the access token as a query parameter.
Currently depends on ad-freiburg/qlever#1691, but it could also be done without this PR by adding the access token to the parameters.
Closes #122.