-
Notifications
You must be signed in to change notification settings - Fork 3
feat (ui): pipeline fetch/publish using mapping set #373
feat (ui): pipeline fetch/publish using mapping set #373
Conversation
aether-ui/aether/ui/api/utils.py
Outdated
if pipeline.mapping_errors: | ||
outcome['error'].append('Mappings have errors') | ||
if contract.mapping_errors: | ||
outcome['error'].append('{} mappings have errors'.format(contract.name)) |
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.
Same comment as in the serializers.py
file... Parlez vous français? Anglais? Espagnol? 🤷♀️
aether-ui/aether/ui/api/utils.py
Outdated
outcome['exists'].append({entity_type['name']: '{} schema with id {} exists'.format( | ||
entity_type['name'], | ||
pipeline.kernel_refs['schema'][entity_type['name']])}) | ||
contract.kernel_refs['schemas'][entity_type['name']])}) |
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.
another styling comment... lines with several indentations are hard to follow.
outcome['exists'].append({
entity_type['name']: '{} schema with id {} exists'.format(
entity_type['name'],
contract.kernel_refs['schemas'][entity_type['name']]),
})
As an example... obviously there are more and better options
aether-ui/aether/ui/api/utils.py
Outdated
|
||
|
||
def publish_preflight(pipeline, project_name, outcome): | ||
def publish_preflight(pipeline, outcome, contract): |
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.
This is checking the pipeline or the contract? Because if it's the contract the pipeline is accessible via contract.pipeline
and you can remove the "duplicated" argument.
Also the outcome could be declared inside and not include as an argument (it's the returned value)
aether-ui/aether/ui/api/utils.py
Outdated
return outcome | ||
|
||
|
||
def publish_pipeline(pipeline, projectname, overwrite=False): | ||
def publish_pipeline(pipeline, project_name, contract, objects_to_overwrite={}): |
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.
Same comment, publishing pipeline and all its contracts or only one contract?
project_id = project.get('id') | ||
project_name = project.get('name') | ||
except Exception: | ||
pass |
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.
I think that the project id should be extracted from the pipeline mapping set and not kept within the contract...
# check linked Project I (using pipeline mapping set)
project_id = None
mappingset_id = None
if pipeline.mappingset:
mappingset_id = pipeline.mappingset
try:
kernel_mappingset = kernel_data_request(f'mappingsets/{pipeline.mappingset}/')
project_id = mappingset.get('project')
# take from kernel_mappingset the data to be push back to kernel
except Exception:
# take from pipeline the data to be pushed to kernel
pass
else:
mappingset_id = str(uuid.uuid4())
# take from pipeline the data to be pushed to kernel
# check linked Project II (using given project name)
if not project_id and project_name:
try:
projects = kernel_data_request(f'projects/?name={project_name}')['results']
aux_project = projects[0] if len(projects) else {}
project_id = aux_project.get('id')
except Exception as e:
pass
# check linked Project III (if everything else failed...)
if not project_id:
project_id = str(uuid.uuid4())
and continue with contract...
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.
The algorithm here is to retrieve the linked project using pipeline.mappingset
or contract.kernel_refs['project']
if previously publish, or create and link an Aux
project to all new pipelines coming from the ui
aether-ui/aether/ui/api/views.py
Outdated
ui_utils.kernel_to_pipeline() | ||
except Exception: | ||
pass | ||
ui_utils.kernel_to_pipeline() | ||
pipelines = models.Pipeline.objects.all() | ||
serialized_data = serializers.PipelineSerializer(pipelines, context={'request': request}, many=True).data | ||
return Response(serialized_data, status=HTTPStatus.OK) |
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.
You can skip these lines just calling the the method list
return self.list(request)
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.
I'll stick with this implementation. Don't want to deal with pagination
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.
You can easily skip pagination with this option 😉
class PipelineViewSet(viewsets.ModelViewSet):
pagination_class = None
The PRO of using return self.list(request)
is that if you have searches/filters/permissions/ordering/... you don't need to implement them twice.
As an example /pipelines/fetch/?fields=id
should return only the list of ids, with your manual approach it doesn't. 🤷♀️
mappingsets_response = kernel_data_request('{}?{}'.format(url_segments.path, url_segments.query)[1:]) | ||
else: | ||
break | ||
for mappingset in mappingsets: |
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.
If you are not using the returned pipelines, you can save memory and resources not creating the array.
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.
Any action here?
@@ -90,6 +78,9 @@ class Pipeline extends Component { | |||
</Link> | |||
<span> // </span> | |||
{ selectedPipeline.name } | |||
{ selectedPipeline.is_read_only && | |||
<span className='tag'>read-only</span> |
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.
I feel like I'm the only one caring about translations... 🙄
@@ -110,6 +113,9 @@ class PipelineList extends Component { | |||
</Modal> | |||
<div | |||
onClick={() => { this.onSelectPipeline(pipeline) }}> | |||
{ pipeline.is_read_only && | |||
<span className='tag'>read-only</span> |
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.
🇩🇪 🇬🇧 🇫🇷 🇪🇸
}) | ||
|
||
export default connect(mapStateToProps, { getPipelines, selectedPipelineChanged, addPipeline, fetchPipelines, getKernelURL })(PipelineList) | ||
export default connect(mapStateToProps, { getPipelines, selectedPipelineChanged, addPipeline, fetchPipelines, getKernelURL, addInitialContract })(PipelineList) |
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.
Do we try to achieve a 1000 chars line? 😧
docker-compose.yml
Outdated
@@ -88,7 +88,7 @@ services: | |||
file: ./docker-compose-base.yml | |||
service: ui-base | |||
depends_on: | |||
db: | |||
kernel: |
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.
Merge conflict?
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.
Should the ui depend on kernel which in turn depends on db or just db?
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.
To avoid "healthy" problems we removed the containers dependencies, only the databases are included, but if you prefer to have it, the most important dependency is the database 😉
ui:
extends:
file: ./docker-compose-base.yml
service: ui-base
depends_on:
db:
condition: service_healthy
kernel:
condition: service_started
# use this dependency with HRM, otherwise comment it out
# ui-assets:
# condition: service_started
networks:
internal:
aliases:
- ui.aether.local
|
||
.pipeline-section__mapping .rule-input { | ||
box-shadow: none; | ||
&.source:before, |
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.
I see sass-lint complaining...
> sass-lint --verbose
./css/_pipelines.scss
100:1 warning Space expected between blocks empty-line-between-blocks
./css/base/_global.scss
236:1 warning Space expected between blocks empty-line-between-blocks
242:1 warning Space expected between blocks empty-line-between-blocks
242:13 warning Pseudo-elements must start with double colons pseudo-element
243:18 warning Pseudo-elements must start with double colons pseudo-element
✖ 5 problems (0 errors, 5 warnings)
…eHealthAfrica/aether into ui-pipeline-publish-mapping-set
{ pipeline.is_read_only && | ||
<span className='tag'> | ||
<FormattedMessage | ||
id='pipeline.navbar.input' |
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.
could you use an id that indicates that the text is "read only"? ;-)
|
||
# Ensure linked mappings are not recreated | ||
# Ensure linked mappings are not recreated] |
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.
😬
Could you merge current |
…a/aether into ui-pipeline-publish-mapping-set
|
||
if outcome['error']: | ||
if 'error' in outcome and len(outcome['error']): |
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.
Simpler: if outcome.get('error'):
😉
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.
I still think that there are a couple of things to be done/rethought... but so far it's great.
👏
* feat: add mapping sets to kernel (#337) * refactor: mapping set model * chore: joining tables - submission, mapping * chore: add input to mapping set model * fix: (kernel-test) mapping set * fix: remove unused containers * fix: db network * fix: set mappingset input to nullable * chore: mappingset migration * fix: (kernel) mapping set to mapping relationship * fix: (kernel) extraction on only active mappings within a set * chore: (kernel) migration clean-up * revert: migration * chore: merge migrations * fix: kernel model update * fix: filter typo * chore: update kernel ERD * chore: migrate mappings to mapping set * fix: clear mapping projectschemas then readd * fix: format code * fix: format code * fix: pylint whitespaces * feat: (ui) mappingsets to pipelines (#361) * feat: ui contract model * fix: kernel migration * fix: kernel migrations * feat: ui mapping set * feat: react component update * test: (ui) pipeline fetch * fix: remove comments * fix: readonly message * fix: model default_name * fix: loop mapping set pages * fix: test add pipeline * fix: artefacts generation with mapping sets (#350) * fix: tweaking models (100% coverage) * fix: tweaking filters (100% coverage) * fix(views): distinct count in stats * fix: upsert project artefacts with mapping sets * fix: API urls * fix(common): submit to kernel with mapping set * fix(odk): submission to kernel * fix(odk): generated mappings are read only * fix: naming naming naming * fix: tweaking * fix(couchdb): adapt to mapping set model * fix: reactivate UI tests in travis (#371) * fix: reactivate UI tests in travis * test: sass-lint rules * fix mappingset migration (#379) * chore: add mappingset to client test_fixtures * chore: swap mapping.id for mappingset.id in submission generation * fix: use model from swagger for submissions in integration tests * fix: remove useless test and change scope of entity generation * feat (ui): pipeline fetch/publish using mapping set (#373) * fix: tweaking models (100% coverage) * fix: tweaking filters (100% coverage) * fix(views): distinct count in stats * fix: upsert project artefacts with mapping sets * fix: API urls * fix(common): submit to kernel with mapping set * fix(odk): submission to kernel * fix(odk): generated mappings are read only * feat: ui contract model * fix: kernel migration * fix: kernel migrations * feat: ui mapping set * feat: react component update * test: (ui) pipeline fetch * fix: remove comments * fix: readonly message * fix: naming naming naming * fix: model default_name * fix: loop mapping set pages * fix: test add pipeline * fix: tweaking * fix: pipeline:contracts * feat: pipeline publish * fix(ui): test * fix: temp deactivate couch-sync tests * test (ui): mapping set 100% * fix: project name * fix: ui test consistency * chore: readonly class * chore: selected pipeline readonly class * added styling for readonly-pipeline in overview screen * added styling to readonly-pipeline navbar * added styling for read-only text inputs * better presentation of mapping-definitions json textarea. * fix (ui): pipeline - contract infix * fix (ui): fix pipeline view * fix(ui): test * fix (ui): check pipelines lenght * fix (ui): migration fix * fix(ui): contract migration * fix: artefacts names (#387) * fix(ui): filter piplines - redux * feat(kernel): create an empty mapping along with the passthrough one (#389) * feat(kernel): create empty mapping * fix: run_entity_extraction * fixed css grid and added word break for long titles without breaking space (#397) * docs(ui): fix model comments (#401) * feat(kernel): include a random input in the generated mapping (#402) * feat: include input in generated mapping * fix: do not duplicate constants * fix(ui): the derived data must have an id field with UUID content (#400) * fix: the schema must have an id field with UUID content * fix: apply only to derived schemas * fix: also derived entity type * fix: cleaning * fix: check id field in EntityTypes list * test: implement id rule * fix: including docs
Fetch kernel artefacts and stores them as
pipeline -> contracts
grouped by mapping setsAllows pipeline create, edit and publish from UI to kernel using mapping sets
Revalidate grouped contracts on pipeline update
Refactor identity mapping to use mapping sets as contracts
Enforce read only pipelines (on input(pipeline) and contract levels individually)
Related JIRA Tickets:
AET-346
AET-342
AET-277
AET-347
AET-343