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.

Sign up for GitHub

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

Jump to bottom

Allow overriding output directory for imports in cdk8s.yaml config file #2159

Open
1 of 2 tasks
abierbaum opened this issue Oct 10, 2020 · 11 comments · May be fixed by cdk8s-team/cdk8s-cli#2937
Open
1 of 2 tasks

Allow overriding output directory for imports in cdk8s.yaml config file #2159

abierbaum opened this issue Oct 10, 2020 · 11 comments · May be fixed by cdk8s-team/cdk8s-cli#2937
Labels
@component/cdk8s-cli Issue related to cdk8s-cli

Comments

@abierbaum
Copy link
Contributor

Description of the feature or enhancement:

I think it would be useful to surface more of the configuration for the import command into the cdk8s.yaml file. This would allow users to declaratively set this in the configuration without having to put it into the command line options in a script or each time they run it.

It appears that code is already in place to do this for 'language'.

https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s-cli/src/cli/cmds/import.ts#L25

This would be extending it to support output and possibly include and exclude.

Use Case:

It would help make it more convenient to use the CLI for import generation and update. It also helps to make clear through configuration what the desired settings for the project are.

Proposed Solution:

Change this line: https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s-cli/src/cli/cmds/import.ts#L22

to something like: default: config.outdir ?? DEFAULT_OUTDIR

I could implement it if there is interest and likelihood of getting it through quickly.

Other:

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request
@eladb
Copy link
Contributor

eladb commented Oct 11, 2020

Sounds good. Happy to take a contribution

@abierbaum
Copy link
Contributor Author

abierbaum commented Oct 13, 2020 •
edited
Loading

@eladb I looked into this more and there is a complication.

Config.output is already defined here: https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s-cli/src/config.ts#L15 and has a default of dist'

export interface Config {
  readonly app?: string;
  readonly language?: Language;
  readonly output?: string;
  readonly imports?: string[];
}

const DEFAULTS: Config = {
  output: 'dist',
};

The synth command relies upon this default and allows it to change in the config file to override it.

See: https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s-cli/src/cli/cmds/synth.ts

So this issue is that both the import and synth commands make use of an output option but it means different things in both places, so I can't easily use it from the configuration file. We could define a new value or values in the configuration file like:

app: node index.js
language: typescript
output: dist
import_output: imports
imports:
  - [email protected]

But that seems ugly and is a pretty big change to put into a small PR.

For now I can just use a script and pass the command line option unless there is something obvious I am missing here.

@pgollucci
Copy link

I agree.

@iliapolo iliapolo transferred this issue from cdk8s-team/cdk8s Jul 6, 2021
@iliapolo iliapolo removed their assignment Jul 8, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2021

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon.

@github-actions
Copy link
Contributor

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen.

@iliapolo iliapolo reopened this Sep 27, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2021

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen.

@github-actions github-actions bot closed this as completed Oct 5, 2021
@iliapolo iliapolo reopened this Oct 5, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2021

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon.

@github-actions
Copy link
Contributor

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen.

@rassie
Copy link

rassie commented Aug 8, 2023

@iliapolo this would still be nice to have, could you reopen this?

@iliapolo iliapolo reopened this Aug 8, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 7, 2024

This issue has not received any attention in 1 year and will be closed soon. If you want to keep it open, please leave a comment below @mentioning a maintainer.

@github-actions github-actions bot added the closing-soon Issue/PR will be closing soon if no response is provided label Aug 7, 2024
@rassie
Copy link

rassie commented Aug 7, 2024

@iliapolo Would still be nice to have. Please keep open.

@github-actions github-actions bot removed the closing-soon Issue/PR will be closing soon if no response is provided label Aug 8, 2024
@iliapolo iliapolo added the @component/cdk8s-cli Issue related to cdk8s-cli label Sep 20, 2024
@iliapolo iliapolo transferred this issue from cdk8s-team/cdk8s-cli Sep 20, 2024
@clayrosenthal clayrosenthal linked a pull request Nov 14, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@component/cdk8s-cli Issue related to cdk8s-cli
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(import): dir config option clayrosenthal/cdk8s-cli
5 participants
@rassie @pgollucci @abierbaum @eladb @iliapolo