-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
chore: update conlinmarc/hdfs to v2.2.1. Fixes #7816 #7826
chore: update conlinmarc/hdfs to v2.2.1. Fixes #7816 #7826
Conversation
21b1033
to
01ac2db
Compare
Signed-off-by: Yashvardhan Kukreja <yash.kukreja.98@gmail.com>
01ac2db
to
7f53928
Compare
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.
Do you have HDFS setup to test out the changes?
@@ -77,7 +75,11 @@ require ( | |||
upper.io/db.v3 v3.6.3+incompatible | |||
) | |||
|
|||
require github.com/go-git/go-git/v5 v5.3.0 | |||
require ( |
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.
I think this should be in the require block above instead of separate.
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.
Sure, I'll do so. The above go.mod related changes were the magic of go mod tidy :)
Have you tested with HDFS env? what is HDFS version? |
Hi @sarabala1979 @terrytangyuan , sorry I was pre-occupied a bit due to which I couldn't test against an existing HDFS setup. I haven't played with the HDFS side of things associated argo-workflows so do you mind sharing template/guide/reference for hdfs<->argo-workflows setup which I can simulate locally and test this PR against ? :) Is this a good place to start - https://github.com/argoproj/argo-workflows/blob/master/examples/hdfs-artifact.yaml ? :) |
@dtaniwaki as the original contributor of hdfs support would you be able to help manually test this PR? |
credentialsv8 "github.com/jcmturner/gokrb5/v8/credentials" | ||
keytabv8 "github.com/jcmturner/gokrb5/v8/keytab" |
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.
nit: previously the version of the package where not part of the name, what is the advantage of adding it?
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.
No certain reason tbh, just kept it at that moment.
I can happily get rid of the v8
s in the naming :)
PS: Apologies for the late reply, my github notifications clutter is a mess to deal with :P
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@blkperl Sorry for the late reply. I’m working in a different org and we don’t have HDFS right now. My current laptop doesn’t have it neither as I don’t use it now… |
Replaced by #11543 |
Signed-off-by: Yashvardhan Kukreja yash.kukreja.98@gmail.com
Fixes #7816
Updates:
colinmark/hdfs v1.1.4
tocolinmark/hdfs/v2 v2.2.1
gopkg.in/jcmturner/gokrb5.v5 v5.3.0
togithub.com/jcmturner/gokrb5/v8 v8.4.2