-
Notifications
You must be signed in to change notification settings - Fork 24
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
import eoapi-cdk constructs and reuse them #144
Conversation
One significant difference with the other PR, in addition, is that I'm not adding the ingestor & browser, I'd like to keep things simple and do that later in another PR if possible. |
🙏 yeah we def want stac-browser and ingestor to be deployed but 👍 for doing it later :-) |
9c24ebe
to
6ca768d
Compare
6ca768d
to
d1f5cfe
Compare
@vincentsarago this is ready to be merged. Important notes :
|
When you're 👍 I can merge and later delete the |
Edit : |
@emileten I'm getting an error at https://ingestor.eoapi.dev/docs is that expected? |
@vincentsarago You can reach /docs with the api gateway url though (not on my computer right now, but that url is one of the outputs of the stack). |
☝️ done
☝️ fixed https://ingestor.eoapi.dev/docs with developmentseed/eoapi-cdk#82 |
infrastructure/aws/cdk/app.py
Outdated
eostac_settings.stac_api_custom_domain_name is not None | ||
), "stac_api_custom_domain_name must be set if stac_browser_github_tag is not None. The browser deployment needs a resolved STAC API url at deployment time and so needs to rely on a predefined custom domain name." | ||
eostac_settings.stac_browser_github_tag is not None | ||
), "stac_browser_github_tag must be set if stac_api_custom_domain_name is not None." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emileten can you explain again the relationship between the custom domain name
and the GitHub tag
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, the way I wrote it, it's confusing.
In eoSTACSettings
, I defined two parameters for the purpose of the STAC browser :
- the catalog URL, which I named
stac_api_custom_domain_name
, that the browser is going to show - the version of the browser,
stac_browser_github_tag
These two things should probably go in another configuration class named eoStacBrowserSettings
for clarity.
And the assertion just checks that a version is provided if a catalog url is provided to avoid unclear errors downstream.
Does that make sense @vincentsarago ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thanks
These two things should probably go in another configuration class named eoStacBrowserSettings for clarity.
That would be lovely
…dd example instead
- add stac browser to docker deployment - make use of radiant earth browser config to have control over which tiler is used - create a separate settings object for the browser for readability
af1f67a
to
edcfeb4
Compare
Another take at #135. This one is much simpler. All modifications are in
infrastructure/aws
, and are all about swapping direct native AWS CDK calls with eoapi-cdk construct calls.There is no change in behavior in this PR ; i.e, the deployed infrastructure should be identical for a given set of environment variables after this PR is merged.
The code in this branch synthesized successfully and I'm trying to deploy right now.