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

Document use of --conf-source option #2187

Merged
merged 8 commits into from
Jan 12, 2023
Merged

Document use of --conf-source option #2187

merged 8 commits into from
Jan 12, 2023

Conversation

ankatiyar
Copy link
Contributor

@ankatiyar ankatiyar commented Jan 10, 2023

Signed-off-by: Ankita Katiyar ankitakatiyar2401@gmail.com

Description

Resolves #2138

Development notes

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Copy link
Contributor

@yetudada yetudada left a comment

Choose a reason for hiding this comment

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

Just left a minor comment. This is great!

ankatiyar and others added 2 commits January 10, 2023 13:39
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@datajoely
Copy link
Contributor

Is it worth documenting the minimum required conf-source directory structure?

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar ankatiyar requested a review from stichbury January 10, 2023 17:10
@@ -8,7 +8,10 @@ We recommend that you keep all configuration files in the `conf` directory of a
```python
CONF_SOURCE = "new_conf"
```

You can also specify a source directory for the configuration files at run time using the `--conf-source` flag with the `kedro run` command as follows:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe include a link here to the commands_reference.md table?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! c1b8e2c

Copy link
Contributor

@stichbury stichbury left a comment

Choose a reason for hiding this comment

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

Nice work, thanks for the additional changes 🤩 🏆

ankatiyar and others added 3 commits January 11, 2023 12:24
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Copy link
Contributor Author

Is it worth documenting the minimum required conf-source directory structure?

@datajoely The section following this in the docs does talk about the conf/base and the conf/local directory structure. Happy to add it in this section as well but it might be a little redundant?

@datajoely
Copy link
Contributor

Is it worth documenting the minimum required conf-source directory structure?

@datajoely The section following this in the docs does talk about the conf/base and the conf/local directory structure. Happy to add it in this section as well but it might be a little redundant?

Oh I didn't realise that - we could link to that anchor possibly, but maybe not needed.

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.

Add documentation for the upcoming --conf-source option for kedro run
5 participants