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

Workflow for mlflow added #74

Closed
wants to merge 4 commits into from

Conversation

Turtle24
Copy link
Contributor

I think I might have a solution for MLflow finally. I've been working with docker quite a bit lately so I think this might work. Tell me what you think.

@Turtle24 Turtle24 force-pushed the mlflow-ci/cd branch 3 times, most recently from 6d0ff6b to 9d98387 Compare September 11, 2021 17:33
@codecov
Copy link

codecov bot commented Sep 11, 2021

Codecov Report

Merging #74 (1de2dd4) into master (e733cb7) will decrease coverage by 6.74%.
The diff coverage is n/a.

❗ Current head 1de2dd4 differs from pull request most recent head 525ad4f. Consider uploading reports for the commit 525ad4f to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##            master      #74      +/-   ##
===========================================
- Coverage   100.00%   93.25%   -6.75%     
===========================================
  Files           19       19              
  Lines          727      727              
===========================================
- Hits           727      678      -49     
- Misses           0       49      +49     
Impacted Files Coverage Δ
sklearn_genetic/callbacks/loggers.py 59.61% <0.00%> (-40.39%) ⬇️
sklearn_genetic/callbacks/base.py 75.00% <0.00%> (-25.00%) ⬇️
sklearn_genetic/space/space.py 80.24% <0.00%> (-19.76%) ⬇️
sklearn_genetic/callbacks/early_stoppers.py 89.70% <0.00%> (-10.30%) ⬇️
sklearn_genetic/callbacks/validations.py 90.00% <0.00%> (-10.00%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e733cb7...525ad4f. Read the comment docs.

@Turtle24 Turtle24 force-pushed the mlflow-ci/cd branch 4 times, most recently from 5cb9baf to 272551f Compare September 11, 2021 20:35
@rodrigo-arenas
Copy link
Owner

Hi, I think is a good idea, there are a couple things I'd to comment:

  • If we get it working on Docker, I think the test_cleanup function would be unnecessary, so we can remove it
  • The test themselves are failing, I think is because the defined docker port is not being reached, please check the execution log in here
    Thanks!

@Turtle24
Copy link
Contributor Author

Hi, I think is a good idea, there are a couple things I'd to comment:

  • If we get it working on Docker, I think the test_cleanup function would be unnecessary, so we can remove it

  • The test themselves are failing, I think is because the defined docker port is not being reached, please check the execution log in here

Thanks!

Hey, yeah you're absolutely right. I'll remove the cleanup and I think I'll use testcontainers to get the server port, there's a compose class that'll actually run the compose.yml for us, which is nice.

No worries!

@Turtle24 Turtle24 force-pushed the mlflow-ci/cd branch 4 times, most recently from 90b3ec5 to 50f7d29 Compare September 18, 2021 09:01
@rodrigo-arenas
Copy link
Owner

Ey thanks, I see now the tests pass, whici is great! I just have to check why the test coverage is decreasing

Sorry for the delayed answer, I was a bit far away a couple weeks

relative imports to avoid mix with installed package
@rodrigo-arenas
Copy link
Owner

Closing for now as test have been successful without docker, just to keep dependencies and test as simple as possible

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