Skip to content
Snippets Groups Projects

Separate connect/read timeouts in pylinkahead.ini

Merged I. Nüske requested to merge f-enh-fit-93-pylinkahead-separate-timeouts into dev
2 unresolved threads

Summary

Addresses https://gitlab.indiscale.com/caosdb/customers/f-fit/management/-/issues/93 / https://gitlab.com/linkahead/linkahead-pylib/-/issues/127
It is now possible to set separate connect/read timeouts or timeout None in pylinkahead.ini.

Corresponding Test-MR: caosdb-pyinttest!86 (merged)

Focus

  • Is the CustomValidator an acceptable solution to the validation of tuples as array?
  • Are there any cornercases where this breaks a request that I missed?

Test Environment

Manual Testing

Check List for the Author

  • All automated tests pass
  • Reference related issues
  • Up-to-date CHANGELOG.md (or not necessary)
  • Up-to-date JSON schema (or not necessary)
  • Appropriate user and developer documentation (or not necessary)
  • Annotations in code (Gitlab comments)

Check List for the Reviewer

  • I understand the intent of this MR
  • All automated tests pass
  • Up-to-date CHANGELOG.md (or not necessary)
  • Appropriate user and developer documentation (or not necessary)
  • The test environment setup works and the intended behavior is reproducible in the test environment
  • In-code documentation and comments are up-to-date.
  • Check: Are there specifications? Are they satisfied?
Edited by Alexander Schlemmer

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
78 87 valobj[s] = {}
79 88 for key, value in config[s].items():
80 89 # TODO: Can the type be inferred from the config object?
81 if key in ["timeout", "debug"]:
90 if key in ["debug"]:
91 valobj[s][key] = int(value)
92 elif key in ["timeout"]:
93 value = "".join(value.split()) # Remove whitespace
94 if str(value).lower() in ["none", "null"]:
95 valobj[s][key] = None
96 elif value.startswith('(') and value.endswith(')'):
97 content = [None if str(s).lower() in ["none", "null"] else int(s)
98 for s in value[1:-1].split(',')]
99 valobj[s][key] = tuple(content)
100 else:
82 101 valobj[s][key] = int(value)
  • I. Nüske
  • I. Nüske
  • 475 # Integer options
    475 476 for opt in int_opts:
    476 477 if opt in global_conf:
    477 478 global_conf[opt] = conf.getint("Connection", opt)
    478 # Boolean options
    479 479
    480 # Boolean options
    480 481 for opt in bool_opts:
    481 482 if opt in global_conf:
    482 483 global_conf[opt] = conf.getboolean("Connection", opt)
    484
    485 # Other options, defer parsing to configuration.config_to_yaml:
    486 connection_config = config_to_yaml(conf)["Connection"]
    487 for opt in other_opts:
    488 if opt in global_conf:
    489 global_conf[opt] = connection_config[opt]
    • Comment on lines -468 to +489
      Author Developer

      The timeout option cannot be retrieved as an int anymore, so it needs to be extracted in a new option category. We reuse the parsing already implemented in config_to_yaml.

      int_opts is now empty, but I did not remove it as we might introduce new options that are integers. Please recheck if this should be removed anyways and re-implemented if needed.

      Additionally, I was wondering whether there are any reasons not to also defer the parsing of int_opts and bool_opts to config_to_yaml - this would have the advantage that all parsing would be done in one method, making future changes easier and the code here simpler. If this would make sense, I can add it to this MR.

    • I agree. I also don't see the motivation why this was split up into two separate places. Maybe you can use git-blame to find the original author and ask, whether there is a good reason to keep it. :-)

    • Please register or sign in to reply
  • I. Nüske
  • I. Nüske marked the checklist item Annotations in code (Gitlab comments) as completed

    marked the checklist item Annotations in code (Gitlab comments) as completed

  • I. Nüske added 1 commit

    added 1 commit

    • 89b25118 - TST: Unittest for config timeout option support of tuples and None

    Compare with previous version

  • mentioned in merge request caosdb-pyinttest!86 (merged)

  • I. Nüske changed the description

    changed the description

  • I. Nüske requested review from @salexan

    requested review from @salexan

  • Alexander Schlemmer
  • Alexander Schlemmer
  • Alexander Schlemmer
  • I. Nüske added 1 commit

    added 1 commit

    Compare with previous version

  • Alexander Schlemmer
  • Alexander Schlemmer marked the checklist item I understand the intent of this MR as completed

    marked the checklist item I understand the intent of this MR as completed

  • Alexander Schlemmer marked the checklist item All automated tests pass as completed

    marked the checklist item All automated tests pass as completed

  • Alexander Schlemmer marked the checklist item Up-to-date CHANGELOG.md (or not necessary) as completed

    marked the checklist item Up-to-date CHANGELOG.md (or not necessary) as completed

  • Alexander Schlemmer marked the checklist item Appropriate user and developer documentation (or not necessary) as completed

    marked the checklist item Appropriate user and developer documentation (or not necessary) as completed

  • Alexander Schlemmer marked the checklist item Appropriate user and developer documentation (or not necessary) as incomplete

    marked the checklist item Appropriate user and developer documentation (or not necessary) as incomplete

  • Alexander Schlemmer
  • Alexander Schlemmer
  • Alexander Schlemmer marked the checklist item Appropriate user and developer documentation (or not necessary) as completed

    marked the checklist item Appropriate user and developer documentation (or not necessary) as completed

  • Alexander Schlemmer marked the checklist item In-code documentation and comments are up-to-date. as completed

    marked the checklist item In-code documentation and comments are up-to-date. as completed

  • Apart from the few minor comments I added, this MR is fine for me and can be merged.

  • Alexander Schlemmer approved this merge request

    approved this merge request

  • I. Nüske added 29 commits

    added 29 commits

    • 6d8b49cc...e9beaf25 - 28 commits from branch dev
    • 8f81981c - Merge branch 'refs/heads/dev' into f-enh-fit-93-pylinkahead-separate-timeouts

    Compare with previous version

  • I. Nüske reset approvals from @salexan by pushing to the branch

    reset approvals from @salexan by pushing to the branch

  • I. Nüske mentioned in commit 600505d1

    mentioned in commit 600505d1

  • merged

  • Please register or sign in to reply
    Loading