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

Remove .osm workflow from workers #227

Merged
merged 8 commits into from
Feb 17, 2022
Merged

Remove .osm workflow from workers #227

merged 8 commits into from
Feb 17, 2022

Conversation

TShapinsky
Copy link
Member

#201
There is still some OSM language existing in alfalfa. Some is dealing with the seed models for OSWs and some is on the front end. I haven't touched the front end due to being unclear as to what is actually referring OSMs and what is an OSW workflow condition that is called "osm".

Copy link
Member

@nllong nllong left a comment

Choose a reason for hiding this comment

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

Looking good. Note that #223 will remove some of the osmName items. I haven't merged yet because we'll need to release an updated version of alfalfa-client.

@@ -141,38 +139,6 @@ def insert_os_tags(self, points_json_path, mapping_json_path):
# add points to database
self.ac.add_site_to_mongo(points_json, self.upload_id)

def add_osm(self):
Copy link
Member

Choose a reason for hiding this comment

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

🧹!

@TShapinsky
Copy link
Member Author

@nllong just merged in the osm rename, everything appears to be working on my end

@nllong
Copy link
Member

nllong commented Feb 3, 2022

@nllong just merged in the osm rename, everything appears to be working on my end

noice! this is going to be an amazing PR with nice cleanup!

@TShapinsky TShapinsky linked an issue Feb 3, 2022 that may be closed by this pull request
@nllong
Copy link
Member

nllong commented Feb 16, 2022

@TShapinsky -- is this still a WIP, or can we merge?

@TShapinsky TShapinsky changed the title [WIP] Remove .osm workflow from workers Remove .osm workflow from workers Feb 16, 2022
@TShapinsky
Copy link
Member Author

@TShapinsky -- is this still a WIP, or can we merge?

I think this is good if we're ready to merge the client change as well

@nllong
Copy link
Member

nllong commented Feb 16, 2022

sweet, let me do a review of it by tomorrow's meeting.

I'm happy merging the client down, but will want to wait to release. Do you think it is an issue having people (via poetry) use the Github version of the client?

@nllong
Copy link
Member

nllong commented Feb 16, 2022

Also, I think the workflow.tar.gz can be removed since it was used in the OSM workflow? True?

That also means def extract_workflow_tar(self): can disappear.

@TShapinsky
Copy link
Member Author

Also, I think the workflow.tar.gz can be removed since it was used in the OSM workflow? True?

That also means def extract_workflow_tar(self): can disappear.

I can make those changes and see if the tests still pass

@TShapinsky
Copy link
Member Author

@nllong we also still have sim_osm.py and step_osm.py and osm_model_advancer should I change all of those to osw?

@nllong
Copy link
Member

nllong commented Feb 16, 2022

@nllong we also still have sim_osm.py and step_osm.py and osm_model_advancer should I change all of those to osw?

I would leave those as is because I'm going to rename the files to just "simulate.py" and "step.py" or something like that. That way the worker "API" just knows to call simulate in the shared mounted directory for FMU or OSWs.

@TShapinsky
Copy link
Member Author

@nllong added the changes, everything appears to still work

@nllong
Copy link
Member

nllong commented Feb 16, 2022

@nllong added the changes, everything appears to still work

sweet, can you fix the pre-commit checks? probably formatting.

@TShapinsky
Copy link
Member Author

@nllong added the changes, everything appears to still work

sweet, can you fix the pre-commit checks? probably formatting.

Looks like I didn't have pre-commit set up on my machine. Should pass now

@nllong nllong self-requested a review February 17, 2022 14:41
Copy link
Member

@nllong nllong left a comment

Choose a reason for hiding this comment

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

looks great! Thanks!

@nllong nllong merged commit 0e1835f into develop Feb 17, 2022
@nllong nllong deleted the 201-remove-osm-workflow branch February 17, 2022 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants