Skip to content

Conversation

@ghostp13409
Copy link
Contributor


Hi @Bowrna

closes: #34838

I've added property-files operator in the spark submit operator.
As I'm a newcomer to this industry I'm not sure if I need to do any other changes other than this. please let me know if there is a need to do any additional changes.

@eladkal eladkal changed the title 34838 adding propertyfiles option Adding property files option in the Spark Submit command Oct 12, 2023
@eladkal
Copy link
Contributor

eladkal commented Oct 12, 2023

can this be tested with unit test?

@ghostp13409
Copy link
Contributor Author

can this be tested with unit test?

Hi @eladkal,

As I mentioned above, I am a newcomer to open source and a also the development as a whole. So I haven't done any testing yet. However I would love to do so if you can guide me on how to do it.

self._hook = self._get_hook()
self._hook.property_files()

def _get_hook(self) -> SparkSubmitHook:
Copy link
Contributor

Choose a reason for hiding this comment

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

you have to accept the property_files param value as one of the parameters that can be used when creating SparkSubmitOperator. This code doesn't involve those changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay will try to add that as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added properties-file param to Operator and Hook files. However, I haven't added the default file path handling logic as it seems to be already handled by spark-submit command as per the docs. I will follow up with the test case changes in the next commit. let me know if there is any feedback or concern.

you have to accept the property_files param value as one of the parameters that can be used when creating SparkSubmitOperator. This code doesn't involve those changes.

@Bowrna
Copy link
Contributor

Bowrna commented Oct 12, 2023

can this be tested with unit test?

Hi @eladkal,

As I mentioned above, I am a newcomer to open source and a also the development as a whole. So I haven't done any testing yet. However I would love to do so if you can guide me on how to do it.

Please check if you can add relevant test in the test_spark_submit.py file for the changes you have done in spark_submit.py file. @ghostp13409

@Bowrna
Copy link
Contributor

Bowrna commented Nov 14, 2023

hi @ghostp13409 any update on this? if you are struck in this let me know... i will see if i can support you. also some static checks are failing. you may need to fix those parts too.

@ghostp13409
Copy link
Contributor Author

hi @ghostp13409 any update on this? if you are struck in this let me know... i will see if i can support you. also some static checks are failing. you may need to fix those parts too.

HI @Bowrna,

thanks for reaching out.

A possible way to make the text more professional is:

I am working on setting up the dev environment, but I am encountering an error while installing breeze. I have tried different platforms (Windows and Linux) and versions of python and pyenv, but the error persists. I would appreciate your guidance on how to resolve this issue, as I have already asked about it on the slack community.

@Bowrna
Copy link
Contributor

Bowrna commented Nov 15, 2023

can you point out where you posted the question? @ghostp13409

@ghostp13409
Copy link
Contributor Author

can you point out where you posted the question? @ghostp13409

I posted my question on #troubleshooting and #random.

@pateash
Copy link
Contributor

pateash commented Nov 28, 2023

Hi,
I have a related issue #35911,
@ghostp13409 if you are stuck, I can pull your branch into mine and merge both changes and raise a PR.
let me know if that works.
cc. @eladkal @Bowrna

@ghostp13409
Copy link
Contributor Author

ghostp13409 commented Nov 28, 2023 via email

@Bowrna
Copy link
Contributor

Bowrna commented Dec 7, 2023

@pateash @ghostp13409 if you need any help to push the fix for this issue, let me know.

@pateash
Copy link
Contributor

pateash commented Dec 11, 2023

#36164
will be replacing this PR
@ghostp13409 I have pulled your commits in, so your contribution will be counted as well.
we can close this PR.

@ghostp13409
Copy link
Contributor Author

#36164
will be replacing this PR
@ghostp13409 I have pulled your commits in, so your contribution will be counted as well.
we can close this PR.

@pateash thanks

@eladkal eladkal closed this Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding property files option in the Spark Submit command

4 participants