You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by "nichtich (via GitHub)" <gi...@apache.org> on 2023/03/09 09:14:07 UTC
[GitHub] [jena] nichtich opened a new pull request, #1792: GH-1778: List graphs without count in Fuseki UI
nichtich opened a new pull request, #1792:
URL: https://github.com/apache/jena/pull/1792
Modified the Fuseki UI to show a list of graphs without triple counts in /edit view by default because counting all graphs is slow and the /edit interface becomes unusable for large numbers for graphs & triples.
GitHub issue resolved #1778
----
- [ ] Tests are included
- [ ] Documentation change and updates are provided for the [Apache Jena website](https://github.com/apache/jena-site/)
- [x] Commits have been squashed to remove intermediate development commit messages.
- [x] Key commit messages start with the issue number (GH-xxxx or JENA-xxxx)
By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the [Contributor's Agreement](https://www.apache.org/licenses/contributor-agreements.html).
----
See the [Apache Jena "Contributing" guide](https://github.com/apache/jena/blob/main/CONTRIBUTING.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1778: List graphs without count in Fuseki UI [jena]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #1792:
URL: https://github.com/apache/jena/pull/1792#issuecomment-1987350206
I think this PR is pending some more discussion, and not yet ready to be merged even adding the tests. But good progress today :wave:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] kinow commented on pull request #1792: GH-1778: List graphs without count in Fuseki UI
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #1792:
URL: https://github.com/apache/jena/pull/1792#issuecomment-1473283275
Got it. Docs are all good. First we migrated the old JQuery and Bootstrap.js code to Vue.js and then added initial unit and e2e tests. Future changes after that are expected to have tests (recent previous pull requests to the UI probably have examples of that).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1778: List graphs without count in Fuseki UI [jena]
Posted by "afs (via GitHub)" <gi...@apache.org>.
afs commented on PR #1792:
URL: https://github.com/apache/jena/pull/1792#issuecomment-1987280384
+1 to listing the graph without counts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] kinow commented on a diff in pull request #1792: GH-1778: List graphs without count in Fuseki UI
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #1792:
URL: https://github.com/apache/jena/pull/1792#discussion_r1148661637
##########
jena-fuseki2/jena-fuseki-ui/src/views/dataset/Edit.vue:
##########
@@ -237,13 +238,19 @@ export default {
this.content = newVal
this.codemirrorEditor.scrollTo(scrollInfo.left, scrollInfo.top)
},
- listCurrentGraphs: async function () {
+ listCurrentGraphs: async function (count=true) {
this.loadingGraphs = true
this.loadingGraph = true
this.code = ''
this.selectedGraph = ''
try {
- this.graphs = await this.$fusekiService.countGraphsTriples(this.datasetName, this.services.query['srv.endpoints'][0])
+ const endpoint = this.services.query['srv.endpoints'][0]
+ if (count) {
+ this.graphs = await this.$fusekiService.countGraphsTriples(this.datasetName, endpoint)
+ } else {
+ const graphs = await this.$fusekiService.listGraphs(this.datasetName, endpoint)
+ this.graphs = Object.fromEntries(graphs.map(name => [name, ""]))
+ }
Review Comment:
I will try to split this function into two later :+1:
##########
jena-fuseki2/jena-fuseki-ui/src/views/dataset/Edit.vue:
##########
@@ -237,13 +238,19 @@ export default {
this.content = newVal
this.codemirrorEditor.scrollTo(scrollInfo.left, scrollInfo.top)
},
- listCurrentGraphs: async function () {
+ listCurrentGraphs: async function (count=true) {
this.loadingGraphs = true
this.loadingGraph = true
this.code = ''
this.selectedGraph = ''
try {
- this.graphs = await this.$fusekiService.countGraphsTriples(this.datasetName, this.services.query['srv.endpoints'][0])
+ const endpoint = this.services.query['srv.endpoints'][0]
Review Comment:
#1679 has some changes for the `FusekiService` and the `src.endpoints`. Once that one is merged, I will rebase here and update the code to that calls `$fusekiService` methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1778: List graphs without count in Fuseki UI [jena]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #1792:
URL: https://github.com/apache/jena/pull/1792#issuecomment-1987254033
I noticed that most PR's were Fuseki-UI, blocked by my. I am sorry :bow: Looking at this one now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1778: List graphs without count in Fuseki UI [jena]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #1792:
URL: https://github.com/apache/jena/pull/1792#issuecomment-1987349948
Alright, I have a local version that contains the function split into two, and I think I understood more about the goal of this issue (also spotted that I didn't add pagination to Edit, only to Info view).
@afs there are a few things to confirm on this PR.
- So this PR disables the `COUNT(*)` queries for the graphs by default
- It eagerly loads the list of graphs once you open the Edit view (even though we still have the button to count graphs)
- What would be the best?
- a. load all graphs by default, without the count once the page is loaded, wait for user to click to count all graphs?
- b. load all graphs by default, without the count once the page is loaded, do not let users click on count (go to Info view instead)
- c. load nothing by default, wait for user to click and then load graphs without count
- Even though we are removing the `COUNT((*)`, that's still required... when you edit the graph, the UI uses that `COUNT` to compare against `MAX_EDITABLE_SIZE`, set in the code to `10000` (from old Fuseki UI)
- Should we let users try to save very large graphs and possibly overload UI and/or backend? i.e. drop the limit/need for count
- Or should we load the count when the user tries to save it (or when s/he clicks on it?
Some screenshots from the current version in this PR:
(when you open the Edit view, it queries the backend when the page is loaded)
(when you click count triples, it's the same as `main` branch)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] nichtich commented on pull request #1792: GH-1778: List graphs without count in Fuseki UI
Posted by "nichtich (via GitHub)" <gi...@apache.org>.
nichtich commented on PR #1792:
URL: https://github.com/apache/jena/pull/1792#issuecomment-1473278977
I created no tests because the current UI was not covered by tests anyway. Same applies to documentation for this feature, it just does not apply to this pull request, so I did not check both checkboxes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] nichtich commented on pull request #1792: GH-1778: List graphs without count in Fuseki UI
Posted by "nichtich (via GitHub)" <gi...@apache.org>.
nichtich commented on PR #1792:
URL: https://github.com/apache/jena/pull/1792#issuecomment-1473663069
I tried to add a basic test (force-push to my fork) but not having dug into Vue testing before I'm stuck with
> [Vue warn]: Failed to resolve component: router-link
and I'm not sure whether the `services` watcher is actually called in the testing environment and what value it should have.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] kinow commented on pull request #1792: GH-1778: List graphs without count in Fuseki UI
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #1792:
URL: https://github.com/apache/jena/pull/1792#issuecomment-1473721228
>I tried to add a basic test (force-push to my fork) but not having dug into Vue testing before I'm stuck with
No worries @nichtich , thanks for providing the initial structure. It's a bit annoying to write these tests without having an existing test to copy (as you noticed, things are not very intuitive, and the tests being added later means you have nothing to base your work on). I can finish the test and ping you if I need help to test/cover some part of your code :+1:
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1778: List graphs without count in Fuseki UI [jena]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #1792:
URL: https://github.com/apache/jena/pull/1792#issuecomment-1987256221
(PR was opened from gbv/ without allowing contributors to modify it, so I cannot rebase it or add commits - ping @nichtich )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
Re: [PR] GH-1778: List graphs without count in Fuseki UI [jena]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #1792:
URL: https://github.com/apache/jena/pull/1792#discussion_r1518885768
##########
jena-fuseki2/jena-fuseki-ui/tests/unit/views/dataset/edit.vue.spec.js:
##########
@@ -0,0 +1,48 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { describe, expect, it } from 'vitest'
+import { mount } from '@vue/test-utils'
+import Edit from '@/views/dataset/Edit.vue'
+
+describe('Edit', () => {
+ const mountFunction = options => {
+ return mount(Edit, {
+ ...options,
+ global: {
+ mocks: {
+ $fusekiService: {
+ async listGraphs(datasetName, endpoint) {
+ return ["default"]
+ }
+ }
+ }
+ }
+ })
+ }
+ it('show default graph without numbers by default', () => {
+ const component = mountFunction({
+ propsData: {
+ datasetName: "myDataSet"
+ }
+ })
+ expect(component.find('.card-body h3').text()).to.equal('Available Graphs')
+
+ // TODO: should return 1 instead
+ // expect(component.findAll('tr').length).to.equal(1)
Review Comment:
:+1:
##########
jena-fuseki2/jena-fuseki-ui/src/services/fuseki.service.js:
##########
@@ -22,6 +22,7 @@ import { BUS } from '@/events'
const DATASET_SIZE_QUERY_1 = 'select (count(*) as ?count) {?s ?p ?o}'
const DATASET_SIZE_QUERY_2 = 'select ?g (count(*) as ?count) {graph ?g {?s ?p ?o}} group by ?g'
+const DATASET_GRAPHS = 'select ?g {graph ?g {}}'
Review Comment:
Maybe we can rename this to `$same_QUERY`...
##########
jena-fuseki2/jena-fuseki-ui/src/services/fuseki.service.js:
##########
@@ -157,6 +158,15 @@ class FusekiService {
return axios.get(this.getFusekiUrl('/$/tasks'))
}
+ async listGraphs (datasetName, endpoint) {
+ const result = await axios.get(this.getFusekiUrl(`/${datasetName}/${endpoint}`), {
+ params: {
+ query: DATASET_GRAPHS
+ }
+ })
+ return ["default", ...result.data.results.bindings.map(binding => binding.g.value)]
Review Comment:
Cleaner than the other methods doing the `forEach`! Will apply that to the rest of the code in a future PR. Thanks!
##########
jena-fuseki2/jena-fuseki-ui/src/views/dataset/Edit.vue:
##########
@@ -237,13 +238,19 @@ export default {
this.content = newVal
this.codemirrorEditor.scrollTo(scrollInfo.left, scrollInfo.top)
},
- listCurrentGraphs: async function () {
+ listCurrentGraphs: async function (count=true) {
this.loadingGraphs = true
this.loadingGraph = true
this.code = ''
this.selectedGraph = ''
try {
- this.graphs = await this.$fusekiService.countGraphsTriples(this.datasetName, this.services.query['srv.endpoints'][0])
+ const endpoint = this.services.query['srv.endpoints'][0]
+ if (count) {
+ this.graphs = await this.$fusekiService.countGraphsTriples(this.datasetName, endpoint)
+ } else {
+ const graphs = await this.$fusekiService.listGraphs(this.datasetName, endpoint)
+ this.graphs = Object.fromEntries(graphs.map(name => [name, ""]))
+ }
Review Comment:
Yeah, it looks like one function with a bool=true returns just the count, and for bool=false the graphs (with the counts too?). I think one is bringing the count to display the graphs and the count. The other is bringing the graphs and services available. If so, we can probably split this function into two, one for the graph services, and another for the counts. (Need to confirm)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org