-
Notifications
You must be signed in to change notification settings - Fork 14
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
Feature/1693 api v3 delete recreate #2055
Conversation
…e contours, empty DatasetControllerV3 to test
- login allowed at /api/login & /api-v3/login - v2/v3 BaseRestApiTest distinquished
…der and no content -> IT updated
…version, includes a sample CR)
…st and post-import + IT to prove correct behavior
…mport + IT to prove correct behavior - fix with common location processing with segment stripping (+normalization)
… GET datasets/{name}/{version}
…/audit-trail added
…ion} now works for # or 'latest' (IT = regression test)
…{name}/{version} and /{name}/latest - improved
…- supports latest for as version-expression, impl for datasets improved by actual existence checking + IT test cases for non-existing/non-latest queries
…properties - supports latest for as version-expression; get impl is unvalidated, put impl checks validity - login is now common, under /api/login for both v2 and v3 (did not work previously)
…properties - supports latest for as version-expression; get impl is unvalidated, put impl checks validity - extended for different validation cases - login is now common, under /api/login for both v2 and v3 (did not work previously)
…ture -> CompletableFuture (mistake reverted)
… added. IT mostly adjusted, but there are todos - DatasetServiceV3 introduced to carry difference in behavior to DatasetService. original entity validation has been divided into create-validation and regular-entity validation. - buildfix for VersionedModelServiceTest
…sion}` in proper validations
…asets/dsName/version/rules, GET datasets/dsName/version/rules/# + IT
- v2 ignores this added information - needs
…api-v3-schema-etc
…api-v3-schema-etc
…ion`. Small updates, thanks @benedeki
… with `MappingTableControllerV3.withMappingTableToResponse`
…3-delete-recreate
% Conflicts: % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/SchemaController.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/SchemaService.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/VersionedModelService.scala % rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/DatasetControllerV3IntegrationSuite.scala
…3-delete-recreate % Conflicts: % rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/DatasetControllerV3IntegrationSuite.scala
% Conflicts: % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/PropertyDefinitionControllerV3.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/v3/MappingTableServiceV3.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/v3/SchemaServiceV3.scala % rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/MappingTableControllerV3IntegrationSuite.scala % rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/PropertyDefinitionControllerV3IntegrationSuite.scala % rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/SchemaControllerV3IntegrationSuite.scala
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.
Tested the disabling and enabling + used-in on name and on name+version.
I am curious. When entity is disable and somebody knows its name he can still "use it" which seems a tad weird. Maybe we should come up with a definition of disabled. Like
- disallow all PUT/POST calls except for PUT on /{name} to enable.
- validate should return
error disabled
so it cannot be run in spark-jobs. - maybe also:
- disallow export
- disallow audit-trail
- on GET /{name} print name, version and disabled true/false (in general not just here)
- on GET /{name}/{version} just print basic info and say it is disabled
Happy to discuss more
...src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala
Show resolved
Hide resolved
...src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala
Show resolved
Hide resolved
...src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala
Show resolved
Hide resolved
...src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala
Show resolved
Hide resolved
We agreed on:
|
…bled` information - mainly on GET ...{/name}
|
…tion. IT updated.
data-model/src/main/scala/za/co/absa/enceladus/model/versionedModel/NamedVersion.scala
Show resolved
Hide resolved
… an error message (`UsedIn` wrapped in `EntityInUseException`)
SonarCloud Quality Gate failed. 0 Bugs No Coverage information |
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.
Nice job, looks good to me. Code reviewed
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.
Reviewed, built, run.
@@ -57,14 +58,12 @@ class DatasetControllerV3 @Autowired()(datasetService: DatasetServiceV3) | |||
case Some((entity, validation)) => |
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.
Shouln't this endpoint have the same validation?
usedIn
functionality both for.../{name}/used-in
and.../{name}/{version}/used-in
existsDELETE .../{name}
, enable for entities viaPUT .../{name}
!isDisabled
check).UsedIn
normalization, empty - V2 had it mixed (sometimesNone
, sometimesSome(Seq.empty)
-> now it allNone
s. + unittestPartially implements #1693
Release notes suggestion
Both
/api-v3/{datasets|schemas|mapping-tables|property-defintions}/{name}/used-in
and/api-v3/{datasets|schemas|mapping-tables|property-defintions}/{name}/{version}/used-in
REST API V3 added, the former is used for disable requests to check the dependencies.UsedIn
normalization introduced.