Skip to content

cli: addition of mapping_type parameter#10

Merged
jirikuncar merged 1 commit intoinveniosoftware:masterfrom
JavierDelgadoFernandez:final_version
Mar 11, 2016
Merged

cli: addition of mapping_type parameter#10
jirikuncar merged 1 commit intoinveniosoftware:masterfrom
JavierDelgadoFernandez:final_version

Conversation

@JavierDelgadoFernandez
Copy link
Contributor

Signed-off-by: Javier Delgado javier.delgado.fernandez@cern.ch

@tiborsimko tiborsimko added this to the Sprint-Record-Data-Model milestone Feb 19, 2016
domapping/cli.py Outdated

mapping = schema_to_mapping(parsed_schema, id, {}, config_instance)
if final:
mapping_name = os.path.splitext(os.path.basename(schema.name))[0]
Copy link
Member

Choose a reason for hiding this comment

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

the file name is not reliable. If the schema is coming from stdin there is no schema.name. This name should be given as an argument to the option.

@JavierDelgadoFernandez JavierDelgadoFernandez changed the title cli: addition of final parameter to schema_to_mapping cli: addition of es_type parameter to schema_to_mapping Feb 22, 2016
@JavierDelgadoFernandez JavierDelgadoFernandez force-pushed the final_version branch 2 times, most recently from 5aa913a to 8919459 Compare February 22, 2016 09:25
domapping/cli.py Outdated
if 'mappings' in parsed_mapping:
result = []
for item in parsed_mapping:
template = mapping_to_jinja(parsed_mapping, item, indent=indent)
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better if by default the type name was the one already in the mappings dict.
What do you think of this workflow?:

  • 1/ The input file is a doctype mapping (no root mappings) => --type option is used and is by default equal to "type"
  • 2/ The input file is a mappings file
    • 2.1/ The input file contains only one doctype mapping => the name of the doctype in the input file is used if the option --type is not set, otherwise the option overrides the type name.
    • 2.2/ The input file contains multiple doctypes => the name used is the one provided in the input file. The command fails if the option --type is set

@JavierDelgadoFernandez JavierDelgadoFernandez force-pushed the final_version branch 2 times, most recently from 27a829c to 8cbb734 Compare February 26, 2016 09:37
@JavierDelgadoFernandez
Copy link
Contributor Author

@nharraud Updated. Please, could you take a look when you have a minute?

assert gen_output == expected


def test_mapping_to_jinja_iconsistencies():
Copy link
Member

Choose a reason for hiding this comment

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

s/test_mapping_to_jinja_iconsistencies/test_mapping_to_jinja_inconsistencies/g

@JavierDelgadoFernandez
Copy link
Contributor Author

@nharraud Could you take a look please?

domapping/cli.py Outdated
@click.option('--indent', '-i', default=4, type=click.INT,
help='Output json indentation step.')
def schema_to_mapping_cli(schema, output, config, indent):
@click.option('--es_type', '-t',
Copy link
Member

Choose a reason for hiding this comment

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

--mapping-type?

@JavierDelgadoFernandez JavierDelgadoFernandez force-pushed the final_version branch 2 times, most recently from 2620e01 to 3aedf55 Compare March 8, 2016 09:26
@JavierDelgadoFernandez
Copy link
Contributor Author

This PR is ready from my side. Could someone take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect property name ('settings' instead of 'mappings')

4 participants