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.

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

Error loading pipeline yaml with empty strings #3642

Closed
danielfleischer opened this issue Nov 29, 2022 · 5 comments · Fixed by #3854
Closed

Error loading pipeline yaml with empty strings #3642

danielfleischer opened this issue Nov 29, 2022 · 5 comments · Fixed by #3854
Assignees
Labels
P1 High priority, add to the next sprint

Comments

@danielfleischer
Copy link
Contributor

YAML supports empty strings as "" or ''. I need to provide an empty string as a parameter to one of the classes. When loading a pipeline from that YAML file using haystack.Pipeline.load_from_yaml I get an exception. This is due to the following code:

if not valid_regex.match(str(pipeline_config)):
because the regex for valid values does not recognize the (pythonic) empty string; top of the file contains its definition

VALID_VALUE_REGEX = re.compile(r"^[-\w/\\.:* \[\]]+$")

Error message

haystack.errors.PipelineConfigError: '' is not a valid variable name or value. Use alphanumeric characters or dash, underscore and colon only.`

Expected behavior
Empty string have their uses and are valid JSON/YAML/Python objects and should be valid as values.

System:

  • Haystack version (commit or version number): 1.11.0

Thanks!

@danielfleischer
Copy link
Contributor Author

Suggestion: change the pattern to re.compile(r"^[-\w/\\.:* \[\]]*$").

@mayankjobanputra
Copy link
Contributor

Hi @danielfleischer, Thanks for pointing out this issue. We are looking into it. Can you also share your exact use case? so that it can help us reproduce this issue?

@danielfleischer
Copy link
Contributor Author

Sure. Simplest example is creating an elasticsearch component:

components:
- name: Store
  params:
    host: localhost
    index: text_1
    password: ""
    port: 80
    username: ""                  
  type: ElasticsearchDocumentStore
- name: Retriever
  params:
    document_store: Store
  type: ElasticsearchRetriever

pipelines:
- name: my_pipeline
  nodes:
  - inputs:
    - Query
    name: Retriever

version: 1.11.0

Thanks for looking into it.

@masci
Copy link
Contributor

masci commented Dec 30, 2022

Going to add this to the backlog.

@danielfleischer your proposed solution seems the right one, feel free to go ahead and open a PR to speed up things.

danielfleischer added a commit to danielfleischer/haystack that referenced this issue Jan 1, 2023
Empty strings are valid yaml/json/python objects and should be allowed in configurations.

Examples (equivalent):
```yaml
password:
password: ""
```

Related to deepset-ai#3642.
@masci masci added the P1 High priority, add to the next sprint label Jan 11, 2023
@ZanSara
Copy link
Contributor

ZanSara commented Jan 11, 2023

@masci fyi about the origin of that regex: #2214 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 High priority, add to the next sprint
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants