Skip to content

Modify schema defaults and their loading#8639

Merged
natefoo merged 3 commits intogalaxyproject:devfrom
jdavcs:sg_config6
Sep 24, 2019
Merged

Modify schema defaults and their loading#8639
natefoo merged 3 commits intogalaxyproject:devfrom
jdavcs:sg_config6

Conversation

@jdavcs
Copy link
Member

@jdavcs jdavcs commented Sep 12, 2019

  • Modify defaults in the schema so that correct values are loaded (add
    notes to description where applicable).
  • When the default in schema was inconcsistent with the default loaded
    in __init__.py, use the latter.
  • Replace kwargs.get(foo, default) with self.foo if default is set
    correctly in the schema.
  • Remove redundant assignments and variables (there are still more
    left).
  • The defalt for citation_cache_lock_dir should be citations/locks.
  • In a few cases where defaults are NOT loaded in __init__.py, and the
    values are clearly meant to be suggested values, move those defaults
    into description.

Ref #8493

Most likely will need to be resolved against #8610

@galaxybot galaxybot added this to the 20.01 milestone Sep 12, 2019
@jdavcs jdavcs added area/admin area/cleanup kind/refactoring cleanup or refactoring of existing code, no functional changes and removed triage labels Sep 12, 2019
@jdavcs
Copy link
Member Author

jdavcs commented Sep 19, 2019

Rebased.

@jdavcs jdavcs added area/configuration Galaxy's configuration system and removed area/admin labels Sep 19, 2019
nsoranzo added a commit to nsoranzo/galaxy that referenced this pull request Sep 20, 2019
for config files.

Broken in galaxyproject#921 .

For context, see galaxyproject#8639 (comment)

Includes also elimination of `resolve_path` function as in
galaxyproject#8646
nsoranzo added a commit to nsoranzo/galaxy that referenced this pull request Sep 20, 2019
for config files.

Broken in galaxyproject#921 .

For context, see galaxyproject#8639 (comment)

Includes also elimination of `resolve_path` function as in
galaxyproject#8646
Copy link
Member

@natefoo natefoo left a comment

Choose a reason for hiding this comment

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

Just a couple minor things and unfortunately it needs another rebase. 😢

@jdavcs
Copy link
Member Author

jdavcs commented Sep 23, 2019

Rebased.

- Modify defaults in the schema so that correct values are loaded (add
  notes to description where applicable).
- When the default in schema was inconcsistent with the default loaded
  in `__init__.py`, use the latter.
- Replace `kwargs.get(foo, default)` with `self.foo` if default is set
  correctly in the schema.
- Remove redundant assignments and variables (there are still more
  left).
- The defalt for `citation_cache_lock_dir` should be `citations/locks`.
- In a few cases where defaults are NOT loaded in `__init__.py`, and the
  values are clearly meant to be suggested values, move those defaults
  into description.
As per discussion in code review.
This is just a hunch. The test shouldn't fail, and it doesn't fail
locally. It seems this might fix it (and point to the cause).
@natefoo natefoo merged commit 0d7df2d into galaxyproject:dev Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/configuration Galaxy's configuration system kind/refactoring cleanup or refactoring of existing code, no functional changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants