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

MAINT: Move schemas to 'schemas/' #945

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

mferrera
Copy link
Collaborator

@mferrera mferrera commented Jan 2, 2025

Toward #895
Stacked on #944

This change expects that the contents of schemas/ is exactly what will be served over the Radix endpoint. It simplifies a few things in order to expand the selection of schemas we will serve, in particular the file schema being created for the inplace volumes product.

@mferrera mferrera marked this pull request as ready for review January 2, 2025 12:46
@mferrera mferrera force-pushed the deploy-schemas-dir-directly branch from da35e09 to 073c56d Compare January 2, 2025 12:47
@mferrera mferrera self-assigned this Jan 2, 2025
@mferrera mferrera force-pushed the deploy-schemas-dir-directly branch 2 times, most recently from 8f7b4f8 to 98412ca Compare January 2, 2025 15:45
This creates a distinct directory that we will serve from the Radix
endpoint. The script that modifies the id when the Docker image starts
is still present, but modified. It will be removed when the schemas URLs
are set from the relevant branch (main for dev, staging for prod).
@mferrera mferrera force-pushed the deploy-schemas-dir-directly branch from 98412ca to fcaafe9 Compare January 2, 2025 15:47
@mferrera mferrera requested review from tnatt and slangeveld January 2, 2025 15:51
@mferrera
Copy link
Collaborator Author

mferrera commented Jan 2, 2025

In #944 I didn't realize I had to update the .dockerignore file as well. It's fixed in this PR.

Copy link
Collaborator

@tnatt tnatt left a comment

Choose a reason for hiding this comment

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

LGTM ☺️

@@ -13,7 +13,7 @@ def test_schema_uptodate():
To update the local schema, run:
`./tools/update_schema`
"""
with open("schema/definitions/0.8.0/schema/fmu_results.json") as f:
with open("schemas/0.8.0/fmu_results.json") as f:
Copy link
Collaborator

Choose a reason for hiding this comment

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

While in here, should we take the version from the FmuResultsSchema.VERSION maybe?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, good point. I'm touching a lot of this code in a PR I'm working on now so I'm going skip it for this one and include in the next one

@mferrera mferrera merged commit 25c7bba into equinor:main Jan 3, 2025
15 checks passed
@mferrera mferrera deleted the deploy-schemas-dir-directly branch January 3, 2025 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants