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.

Sign up for GitHub

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

Jump to bottom

fix: download linked objects in a WFS using "resolvedepth" #1180

Merged
merged 1 commit into from
Sep 5, 2024
Merged

fix: download linked objects in a WFS using "resolvedepth" #1180

merged 1 commit into from
Sep 5, 2024

Conversation

emanuelaepure10
Copy link
Contributor

@emanuelaepure10 emanuelaepure10 commented May 21, 2024 •
edited
Loading

Using "resolvedepth" when trying to download data via WFS is now downloading the linked objects. As the requested hits to a WFS is better to be setup to UNKNOWN_SIZE, so the size comes as well to be always UNKNOWN_SIZE, the code related to hits and size has been removed and the related methods have been changed accordingly.

ING-4128
Closes #1084

@emanuelaepure10 emanuelaepure10 force-pushed the fix/ING-4128 branch 2 times, most recently from fc243b7 to ef11840 Compare May 22, 2024 11:56
@emanuelaepure10
Copy link
Contributor Author

@stempler Any idea why my PRs are always failing with the same error in the "Comment with links to the artifacts"?
Thank you

@stempler stempler force-pushed the fix/ING-4128 branch 2 times, most recently from cadb5a6 to 5519274 Compare May 22, 2024 14:41
florianesser
florianesser requested changes May 22, 2024
View reviewed changes
Copy link
Member

@florianesser florianesser left a comment

Choose a reason for hiding this comment

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

Commit message and PR should reference the related GitHub issue #1084

@emanuelaepure10
Copy link
Contributor Author

Or if we just setup the hits to UNKNOWN_SIZE, then the ignoreNumberMatched box doesn't make any sense because wont be used.

@emanuelaepure10 emanuelaepure10 force-pushed the fix/ING-4128 branch 6 times, most recently from 70b72af to 91bac9e Compare May 28, 2024 15:18
@emanuelaepure10
Copy link
Contributor Author

@florianesser I have squashed the previous commit and this one just to be able to better see the change.

florianesser
florianesser approved these changes May 29, 2024
View reviewed changes
Copy link
Member

@florianesser florianesser left a comment

Choose a reason for hiding this comment

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

LGTM

stempler
stempler reviewed Jun 18, 2024
View reviewed changes
...boldt.hale.io.gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/GmlInstanceCollection.java Outdated Show resolved Hide resolved
...boldt.hale.io.gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/GmlInstanceCollection.java Outdated Show resolved Hide resolved
....gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/wfs/WfsBackedGmlInstanceCollection.java Outdated Show resolved Hide resolved
....gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/wfs/WfsBackedGmlInstanceCollection.java Outdated Show resolved Hide resolved
....gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/wfs/WfsBackedGmlInstanceCollection.java Show resolved Hide resolved
....gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/wfs/WfsBackedGmlInstanceCollection.java Outdated Show resolved Hide resolved
@emanuelaepure10
Copy link
Contributor Author

@stempler does it happened to remember if there is somewhere a test example that contains "additionalObjects"? Thanks

@emanuelaepure10
Copy link
Contributor Author

@stempler that should be hopefully the last and complete version. Please let me know if there is anything else to add.
Thank you for your support

stempler
stempler requested changes Aug 15, 2024
View reviewed changes
...boldt.hale.io.gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/GmlInstanceCollection.java Outdated Show resolved Hide resolved
...boldt.hale.io.gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/GmlInstanceCollection.java Outdated Show resolved Hide resolved
...e.io.gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/wfs/DuplicateIDsFilterIterator.java Outdated Show resolved Hide resolved
...e.io.gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/wfs/DuplicateIDsFilterIterator.java Show resolved Hide resolved
...e.io.gml/src/eu/esdihumboldt/hale/io/gml/reader/internal/wfs/DuplicateIDsFilterIterator.java Outdated Show resolved Hide resolved
....hale.io.gml.test/src/eu/esdihumboldt/hale/io/gml/reader/internal/StreamGmlReaderTest.groovy Show resolved Hide resolved
...esdihumboldt.hale.io.gml.geometry/src/eu/esdihumboldt/hale/io/gml/geometry/GMLConstants.java Outdated Show resolved Hide resolved
@emanuelaepure10 emanuelaepure10 force-pushed the fix/ING-4128 branch 2 times, most recently from bc581c7 to 477e506 Compare August 16, 2024 06:56
@emanuelaepure10
Copy link
Contributor Author

It seems that the following three tests are failing:

StreamGmlWriterTest.testGeometryPrimitive_32_MultiPolygon_WindingOrder_CW
StreamGmlWriterTest.testGeometryPrimitive_32_MultiPolygon_WindingOrder 
StreamGmlWriterTest.testGeometryPrimitive_32_MultiPolygon_WindingOrder_CCW

However, these tests are passing successfully on my local environment.

Do you have any suggestions on what might be causing the failures and how to resolve them?

@emanuelaepure10
Copy link
Contributor Author

It appears that 3 tests are failing.
When I run the tests locally, all 16 tests in the StreamGmlWriterTest class pass successfully.

Could you provide some guidance on what might be causing these failures and how I can resolve them?

@stempler
Copy link
Member

stempler commented Aug 20, 2024 •
edited
Loading

Could you provide some guidance on what might be causing these failures and how I can resolve them?

No clue at the moment what could be the issue. I retriggered the check but same result.
I ported the change to hale-core, I don't have the result on the CI yet, but locally these three tests also failed.

It seems to be a side effect of other tests running:

  • if I only run one of the tests it works
  • if I run only this test class it works
  • if I run all tests from this project these three tests fail (Update: but not every time)

@emanuelaepure10 emanuelaepure10 force-pushed the fix/ING-4128 branch 3 times, most recently from b96fa31 to 114f046 Compare August 20, 2024 20:11
@emanuelaepure10
Copy link
Contributor Author

test this please

@stempler
Copy link
Member

To rerun the GitHub actions job when there were failures you can just open the workflow run and select "Rerun jobs" -> "Rerun failed jobs".

@emanuelaepure10
Copy link
Contributor Author

emanuelaepure10 commented Aug 21, 2024 •
edited
Loading

@stempler @florianesser
I temporarily removed the test in StreamGmlReaderTest', which was already commented as something to be moved when the services were no longer functional. This seems to have resolved the issue. However, I still don’t see how this is related to the failure of the other three tests in StreamGmlWriterTest.

LE: I was wrong. Sorry for disturbing, the error is still there.

@emanuelaepure10 emanuelaepure10 force-pushed the fix/ING-4128 branch 3 times, most recently from 9358310 to 1df2898 Compare August 21, 2024 14:45
@emanuelaepure10
fix: download linked objects in a WFS using "resolvedepth"
9c8eb15 
Using "resolvedepth" when trying to download data via WFS is now downloading all the linked objects.

ING-4128
Closes halestudio#1084
@stempler stempler added the challenged For PRs to indicate that the implementation has been challenged label Sep 5, 2024
@stempler stempler merged commit 35c5f85 into halestudio:master Sep 5, 2024
8 of 9 checks passed
@stempler stempler deleted the fix/ING-4128 branch September 5, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@florianesser florianesser florianesser approved these changes

@stempler stempler stempler approved these changes

Assignees
No one assigned
Labels
challenged For PRs to indicate that the implementation has been challenged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG]: If downloading data via WFS with parameter "resolvedepth" linked objects are not loaded
3 participants
@emanuelaepure10 @stempler @florianesser