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

feat(hue): enable application log on jobbrowser of hue #800

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

nschung
Copy link
Contributor

@nschung nschung commented Sep 28, 2023

Which issue(s) this PR fixes

Give hue user access to yarn and spark application log

Additional comments

Agreements

@gonzaloetjo
Copy link
Contributor

gonzaloetjo commented Sep 29, 2023

I'm wondering of the implications of adding additional extras components to tdp-collections. Would it be more appropriate to introduce this as an override within tdp-getting-started, as it's pertinent to a specific implementation with some extra components? Instead of adding extras dependencies on collections.

Additionally, I was evaluating the possibility of having a dedicated overrides directory for tdp-collections-extras (or similar community collections). This would allow extras to inject variables into tdp-collections more seamlessly. The tdp init process could then prioritize these overrides before deployment. What are your thoughts on this approach?

@rpignolet
Copy link
Contributor

rpignolet commented Sep 29, 2023

We cannot add a value to a string with the current override mechanism. This string would have to be a dict if we wanted to do it.

@gonzaloetjo
Copy link
Contributor

I was leaning more towards the idea of overriding the entire key using an tdp-collection-extras overrides folder, which would contain the additions and the original values, similar to those in this commit. A dict might be a solution to.

@rpignolet
Copy link
Contributor

I think we should not duplicate a variable from tdp-collection in tdp-collection-extra. In the case of modifying the variable in tdp-collection, if we forget to make the modification in tdp-collection-extra this will cause us problems. We must stick to the behavior that tdp-collection-extra adds variables or adds elements into an existing variable but not a complete override.

If Hue is not deployed in the cluster, I think it is not a concern for it to be present in the configuration in tdp-collection unless it prevents Hadoop from working.

The alternative would be to change these variables to dict but I don't think we gain anything there.

@gonzaloetjo
Copy link
Contributor

gonzaloetjo commented Sep 29, 2023

The alternative would be to change these variables to dict but I don't think we gain anything there.

IMO we gain for the-collection-extras, or any other community collection, to be independent from tdp-collections, as more and more external variables might be declared in tdp-collection. But that's about it agreed.

@rpignolet
Copy link
Contributor

I agree to make dicts but how can we know that certain variable must be a dict? Do we look for all the variables to put them in dict or do we change them to dict only if necessary?

@rpignolet
Copy link
Contributor

As discuss, will be done in #801.

@rpignolet rpignolet merged commit ea049b1 into master Sep 29, 2023
3 checks passed
@rpignolet rpignolet deleted the configure-app-log-hue branch September 29, 2023 13:37
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.

4 participants