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

add except, update changelog #453

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

sami-sweng
Copy link

Description

Why was this PR created?

Development notes

What have you changed, and how has this been tested?

Checklist

  • Read the contributing guidelines
  • Open this PR as a 'Draft Pull Request' if it is work-in-progress
  • Update the documentation to reflect the code changes
  • Add a description of this change and add your name to the list of supporting contributions in the CHANGELOG.md file. Please respect Keep a Changelog guidelines.
  • Add tests to cover your changes

Notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.

  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.

  • I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

@sami-sweng
Copy link
Author

@Galileo-Galilei I can't see what's failing in Snyk, no access to the org. Should I request access?

@Galileo-Galilei
Copy link
Owner

No don't worry. It's failing for a while because of mlflow dependencies issues.
Thanks for the PR, I'll have a look soon!

@sami-sweng
Copy link
Author

Okay, thank you. I can say that I tested with our internal project and building it with kedro-mlflow from this commit and I don't have any further error. So it looks that was the only issue from our perspective with version 0.18.1.

The only issue I can see is that using 0.18.1 would prevent users from configuring with something others than default. But I'm not sure how to proceed with this and since at least the basic functionality is fixed with this PR, adding config support seems less urgent and could be provided in a separate PR.

Copy link
Owner

@Galileo-Galilei Galileo-Galilei left a comment

Choose a reason for hiding this comment

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

Almost good to go, all the failing tests do not come from you. I'll merge and realease as soon as the changelog is modified. Thank you very much for the PR and sorry for the delay!

CHANGELOG.md Outdated Show resolved Hide resolved
@Galileo-Galilei
Copy link
Owner

(And if I remember well, the kedro ==0.18.1 version is a but buggy, you should upgrade if possible)

@Galileo-Galilei Galileo-Galilei merged commit be66875 into Galileo-Galilei:master Oct 3, 2023
0 of 12 checks passed
@sami-sweng
Copy link
Author

Thank you for the merge!

Do you plan to release soon? I ask but it's not my goal to add any pressure, it's to know if you plan to add other commits in this release I will host the fix on a private pypi server in the meantime while if you plan to do it in the next few days I'll simply wait on the official version.

Thanks for the great work on this lib!

@Galileo-Galilei
Copy link
Owner

Galileo-Galilei commented Oct 3, 2023

It's on PyPI! (but I really recommend you upgrade your kedro version if you can)

@sami-sweng
Copy link
Author

Excellent, thanks a lot. Yes I'll also check about updating kedo, thanks for the tip!

Galileo-Galilei added a commit that referenced this pull request Dec 19, 2023
…ompatibility (#480)

Signed-off-by: Yolan Honoré-Rougé <yolan.honore.rouge@gmail.com>
Galileo-Galilei added a commit that referenced this pull request Dec 19, 2023
…ompatibility (#480)

Signed-off-by: Yolan Honoré-Rougé <yolan.honore.rouge@gmail.com>
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.

None yet

2 participants