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

Datafeeder : Allow import of csv files #4137

Merged
merged 3 commits into from
Jun 11, 2024
Merged

Datafeeder : Allow import of csv files #4137

merged 3 commits into from
Jun 11, 2024

Conversation

f-necas
Copy link
Contributor

@f-necas f-necas commented Jan 5, 2024 •
edited

CSV in datafeeder

Allows to import CSV geo and non geo data in datafeeder.

For the front part, see PR : georchestra/geonetwork-ui#9

Upsteam PR : geonetwork/geonetwork-ui#773

Screenshot

image

@pmauduit pmauduit force-pushed the DF/csv-in-datafeeder branch 2 times, most recently from 10ac6da to 2b31eb7 Compare May 3, 2024 09:29
pmauduit
pmauduit reviewed May 10, 2024
View reviewed changes
datafeeder/pom.xml Outdated Show resolved Hide resolved
@f-necas f-necas added this to the 24.0.0 milestone May 16, 2024
@f-necas
Copy link
Contributor Author

f-necas commented May 20, 2024

Force pushed squashed commits. See branch old-csv-support for full commits history

@f-necas f-necas marked this pull request as ready for review May 20, 2024 06:52
@f-necas f-necas mentioned this pull request May 20, 2024
Open
groldan
groldan requested changes May 28, 2024
View reviewed changes
Copy link
Member

@groldan groldan left a comment

Choose a reason for hiding this comment

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

@f-necas this looks good overall.
Please address the pending change request (curl example and pom), and squash into a single commit with a good title and description.
Also there's a test failure, we'd need to address that.

datafeeder/pom.xml Outdated Show resolved Hide resolved
datafeeder/README.md Outdated Show resolved Hide resolved
datafeeder/pom.xml Outdated Show resolved Hide resolved
@f-necas f-necas force-pushed the DF/csv-in-datafeeder branch 3 times, most recently from fa0ae63 to b81410a Compare June 4, 2024 14:20
@fvanderbiest
Copy link
Member

The PR should include a line in the release notes to explain that the dataset name is now generated from the dataset title rather than the original file name. This is an important change.

f-necas and others added 3 commits June 11, 2024 15:09
@f-necas @pmauduit
datafeeder - add CSV support to datafeeder
46199f7 
This adds basic CSV support to the datafeeder, allowing users to send
geo or non-geo CSV datasets, in addition to the shapefile support.

Considering the ogc api features (georchestra/data-api) project, we can
also add links to the data-api.

Since we can now upload non-geo datasets, it can be sometimes irrelevant
to publish metadata containing links to wfs/wms resources or extent
infos.

This required to bump geotools to v29.

Tests:
* fixing ITs,
* runtime tested,
* Some related tests added to the testsuite
@pmauduit
spring-integration - avoid prefixing roles twice
5c94090 
If the sec-user base64-encode json header is received with already
prefixed roles, one want to avoid prefixing another time, ending up with
a list of roles containing "ROLE_ROLE_..." named roles.

tests: adding an UT.
@pmauduit
df - explicitely switch to java 11
42f4806
@pmauduit
Copy link
Member

pmauduit commented Jun 11, 2024 •
edited

The PR should include a line in the release notes to explain that the dataset name is now generated from the dataset title rather than the original file name. This is an important change.

This has been done (in a dedicated PR though): https://github.com/georchestra/georchestra/pull/4188/files#diff-fa9ce18bbd795fbe01c3be557f7253cbb15ef64db8e5e94e97ac8bdcdd38a7d4R102

This was referenced Jun 11, 2024
Open
Closed
@pmauduit
Copy link
Member

IT testsuite is a bit flacky, because it seems to time-out from time to time. No issue launching it locally, the GHA actually went green at the 4th try, taking 12+ minutes to run, I had run it locally in less than 2 minutes, so probably a matter of available resources ? Considering a large amount of resources is needed to run the suite.

@pmauduit pmauduit merged commit 7597aef into master Jun 11, 2024
6 of 7 checks passed
@pmauduit pmauduit deleted the DF/csv-in-datafeeder branch June 11, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmauduit pmauduit pmauduit left review comments

@groldan groldan groldan requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
24.0.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@f-necas @fvanderbiest @pmauduit @groldan