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

Configurable servers #312

Merged
merged 14 commits into from
Oct 2, 2023
Merged

Configurable servers #312

merged 14 commits into from
Oct 2, 2023

Conversation

Zeitsperre
Copy link
Member

@Zeitsperre Zeitsperre commented Sep 22, 2023

FYI @fmigneault

Changes

  • The THREDDS and GeoServer instances can now be configured for staging/testing purposes. These new environment variables are documented within the installation docs:
    • RAVENPY_THREDDS_URL: URL to the THREDDS-hosted climate data service. Defaults to https://pavics.ouranos.ca/twitcher/ows/proxy/thredds.
    • RAVENPY_GEOSERVER_URL: URL to the GeoServer-hosted vector/raster data. Defaults to https://pavics.ouranos.ca/geoserver.
      • This environment variable GEO_URL is still valid for legacy purposes.
  • The _determine_upstream_ids function under ravenpy.utilities.geoserver was a direct copy of determine_upstream_ids found in ravenpy.utilities.geo and has been removed.
  • The get_CASPAR_dataset and get_ECCC_dataset functions now accept a thredds and a directory in their call signatures for specifying file location.
  • Pins xarray below 2023.09.0 until xclim addresses compatibility issues (will be fixed in 0.46.0).

Relevant discussion:

bird-house/birdhouse-deploy#14

@Zeitsperre Zeitsperre added documentation Improvements or additions to documentation enhancement New feature or request labels Sep 22, 2023
@Zeitsperre Zeitsperre requested a review from huard September 22, 2023 15:42
@Zeitsperre Zeitsperre self-assigned this Sep 22, 2023
@coveralls
Copy link

coveralls commented Sep 22, 2023

Pull Request Test Coverage Report for Build 6352852909

  • 31 of 50 (62.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on configurable-servers at 81.16%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ravenpy/extractors/forecasts.py 11 16 68.75%
ravenpy/utilities/forecasting.py 6 12 50.0%
ravenpy/utilities/geoserver.py 14 22 63.64%
Totals Coverage Status
Change from base Build 6326571058: 81.2%
Covered Lines: 3317
Relevant Lines: 4087

💛 - Coveralls

Copy link

@fmigneault fmigneault left a comment

Choose a reason for hiding this comment

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

Not sure how it relates to bird-house/birdhouse-deploy#14 for the WPS response URLs?

ravenpy/extractors/forecasts.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@tlvu tlvu left a comment

Choose a reason for hiding this comment

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

Very nice now that both Thredds and Geoserver are configurable. Simply keep the backward-compat with the old public configGEO_URL please.

ravenpy/extractors/forecasts.py Show resolved Hide resolved
ravenpy/utilities/geoserver.py Show resolved Hide resolved
@Zeitsperre Zeitsperre requested a review from tlvu September 27, 2023 17:29
@Zeitsperre
Copy link
Member Author

Zeitsperre commented Sep 27, 2023

@huard Builds appear to be failing due to SDBA-related changes. Could it be the latest scipy (10 days ago)?

EDIT: Nvm, this is coming from the new xarray alongside xclim. The fix is already here: Ouranosinc/xclim@c846ca6

Copy link
Collaborator

@tlvu tlvu left a comment

Choose a reason for hiding this comment

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

LGTM. We should always try to minimize breaking changes that are backward incompatible.

HISTORY.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@tlvu tlvu left a comment

Choose a reason for hiding this comment

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

LGTM

@Zeitsperre Zeitsperre merged commit 5fbb43f into master Oct 2, 2023
14 checks passed
@Zeitsperre Zeitsperre deleted the configurable-servers branch October 2, 2023 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants