Skip to content

MINOR: Extend release.py with a subcommand for staging docs into the kafka-site repo#3917

Closed
ewencp wants to merge 3 commits into
apache:trunkfrom
ewencp:stage-docs
Closed

MINOR: Extend release.py with a subcommand for staging docs into the kafka-site repo#3917
ewencp wants to merge 3 commits into
apache:trunkfrom
ewencp:stage-docs

Conversation

@ewencp

@ewencp ewencp commented Sep 20, 2017

Copy link
Copy Markdown
Contributor

No description provided.

@ewencp

ewencp commented Sep 20, 2017

Copy link
Copy Markdown
Contributor Author

@ijuma @dguy @gwenshap Doing this manually was becoming really annoying and I know at least streams has had something of an uptick in frequency of these changes, including on already released branches. For people writing patches, with the right organization of checkouts, it can help them validate their PRs. For reviewers, it lets you validate docs changes in this repo against the full site setup more easily, then do the full merge to kafka, generate docs, merge to kafka-site process more easily.

@dguy

dguy commented Sep 20, 2017

Copy link
Copy Markdown
Contributor

Thanks @ewencp, sounds like a great idea! However my python is pretty terrible, so I can't really offer much in terms of a review of the code!

@ijuma ijuma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is awesome @ewencp. I ran into an issue when testing. See below.

Comment thread release.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, doesn't seem like this is correct.

version ['1', '0', '0-SNAPSHOT']
major_minor ['1', '0']
Extracting ['tar', 'xf', '/Users/ijuma/src/kafka/core/build/distributions/kafka_2.11-1.0.0-SNAPSHOT-site-docs.tgz', '--strip-components', '1']
Traceback (most recent call last):
File "./release.py", line 235, in
command_stage_docs()
File "./release.py", line 227, in command_stage_docs
cmd('Extracting ', 'tar xf %s --strip-components 1' % docs_tar, cwd=os.path.join(kafka_site_repo_path, docs_version(version)))
File "./release.py", line 108, in cmd
output = subprocess.check_output(cmd, *args, stderr=subprocess.STDOUT, **kwargs)
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 566, in check_output
process = Popen(stdout=PIPE, *popenargs, **kwargs)
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 710, in init
errread, errwrite)
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1335, in _execute_child
raise child_exception
OSError: [Errno 2] No such file or directory: '/Users/ijuma/src/kafka/../kafka-site/10'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, maybe this is correct and the issue is just that the code must handle the case where the target directory doesn't exist?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, it seems like our convention for docs versions is quite bad. If we didn't want to use ., we could use something else like _.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, likely the missing directory. I agree the docs versioning style is pretty terrible. 1.0 would be a good opportunity to change it as it makes for an easy way to detect which mode you should be using.

Possibly @derrickdoo will have some thoughts. I'm not sure how painful it would be given other places the version appears and the stuff we're doing w/ js now.

Comment thread release.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is also:

docs_version = ''.join(release_version_parts[:3])

Worth consolidating?

@ewencp

ewencp commented Feb 21, 2018

Copy link
Copy Markdown
Contributor Author

I think all the issues have been addressed (as well as fixing to also copy the javadocs, which I had missed before).

@ewencp

ewencp commented Feb 26, 2018

Copy link
Copy Markdown
Contributor Author

Ping again, would like to get this merged as it's pretty painful trying to test changes to the docs tooling and styling without this.

@dguy dguy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@ewencp ewencp closed this in f582a75 Feb 28, 2018
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.

3 participants