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

fix: Add mongodb env variables to ingest-test-fixtures-update-pr.yaml #2851

Merged
merged 5 commits into from
Apr 5, 2024
Merged

fix: Add mongodb env variables to ingest-test-fixtures-update-pr.yaml #2851

merged 5 commits into from
Apr 5, 2024

Conversation

potter-potter
Copy link
Contributor

ingest-test-fixtures-update-pr.yaml was missing mongodb vars. And the workflow was failing.

@ryannikolaidis
Copy link
Contributor

ryannikolaidis commented Apr 4, 2024 •
edited
Loading

curious, why wasn't CI failing?

@potter-potter potter-potter requested a review from scanny April 4, 2024 22:14
@ryannikolaidis ryannikolaidis self-requested a review April 4, 2024 22:14
@ryannikolaidis
Copy link
Contributor

ohh this is the update fixtures, got it!

ryannikolaidis
ryannikolaidis reviewed Apr 4, 2024
View reviewed changes
CHANGELOG.md Outdated
@@ -12,6 +12,7 @@

* **Fix `partition_html()` swallowing some paragraphs**. The `partition_html()` only considers elements with limited depth to avoid becoming the text representation of a giant div. This fix increases the limit value.
* **Fix SFTP** Adds flag options to SFTP connector on whether to use ssh keys / agent, with flag values defaulting to False. This is to prevent looking for ssh files when using username and password. Currently, username and password are required, making that always the case.
* **Fix Add Mongodb Env Vars to ingest-test-fixtures-update-pr** Mongodb env vars were never added. This caused errors when trying to update test fixtures.
Copy link
Contributor

@ryannikolaidis ryannikolaidis Apr 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

non-application / CI changes don't need to bump the changelog

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ryannikolaidis I removed all changes to changelog and __version__. I hope thats what you meant.

Copy link
Contributor

Choose a reason for hiding this comment

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

yep, thank you!

scanny
scanny approved these changes Apr 4, 2024
View reviewed changes
Copy link
Collaborator

@scanny scanny left a comment

Choose a reason for hiding this comment

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

LGTM :)

ryannikolaidis
ryannikolaidis approved these changes Apr 4, 2024
View reviewed changes
Copy link
Contributor

@ryannikolaidis ryannikolaidis left a comment

Choose a reason for hiding this comment

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

nit: can we not bump the changelog since this doesn't impact the end user?

@potter-potter potter-potter added this pull request to the merge queue Apr 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 4, 2024
@scanny
Copy link
Collaborator

scanny commented Apr 4, 2024 •
edited
Loading

nit: can we not bump the changelog since this doesn't impact the end user?

@ryannikolaidis I believe the Changelog workflow will fail if there are no changes to CHANGELOG and if __version__ does not correspond. So the minimal change to get a mergeable PR is to bump the changelog (version number) and update __version__ to match.

At least that's the way it was when I formed that habit months ago :)

@potter-potter potter-potter added this pull request to the merge queue Apr 4, 2024
@potter-potter potter-potter removed this pull request from the merge queue due to a manual request Apr 4, 2024
@scanny scanny added this pull request to the merge queue Apr 4, 2024
@potter-potter
Copy link
Contributor Author

potter-potter commented Apr 4, 2024 •
edited
Loading

nit: can we not bump the changelog since this doesn't impact the end user?

@ryannikolaidis I believe the Changelog workflow will fail if there are no changes to CHANGELOG and if __version__ does not correspond. So the minimal change to get a mergeable PR is to bump the changelog (version number) and update version to match.

At least that's the way it was when I formed that habit months ago :)

Thats what I thought. Looks like CI didn't like it without one.
there is already a commit associated with version 0.13.1.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 4, 2024
@scanny
Copy link
Collaborator

scanny commented Apr 4, 2024

@potter-potter you need to bump the version number in CHANGELOG and change the version in unstructured/__version__.py to match it. No need for an actual CHANGELOG "line-item" if it doesn't affect the user, but you do need to change the version.

In this case, since Ryan just cut a new release, you need to add a new version block named 0.13.2-dev0 with empty Enhancements, Fixes, etc. blocks underneath it.

@ryannikolaidis
Copy link
Contributor

@potter-potter you need to bump the version number in CHANGELOG and change the version in unstructured/__version__.py to match it. No need for an actual CHANGELOG "line-item" if it doesn't affect the user, but you do need to change the version.

In this case, since Ryan just cut a new release, you need to add a new version block named 0.13.2-dev0 with empty Enhancements, Fixes, etc. blocks underneath it.

Yep! That ☝️ exactly.

@scanny scanny enabled auto-merge April 4, 2024 23:18
@scanny scanny added this pull request to the merge queue Apr 4, 2024
Merged via the queue into main with commit 57c7c7a Apr 5, 2024
46 checks passed
@scanny scanny deleted the potter/add-mongodb-env-variables branch April 5, 2024 00:12
kaaloo pushed a commit to inclusif/unstructured that referenced this pull request Apr 8, 2024
@potter-potter @kaaloo
fix: Add mongodb env variables to ingest-test-fixtures-update-pr.yaml (…
9055ae5 
…Unstructured-IO#2851)

ingest-test-fixtures-update-pr.yaml was missing mongodb vars. And the
workflow was failing.
kaaloo pushed a commit to inclusif/unstructured that referenced this pull request Apr 8, 2024
@potter-potter @kaaloo
fix: Add mongodb env variables to ingest-test-fixtures-update-pr.yaml (…
ff3e5e6 
…Unstructured-IO#2851)

ingest-test-fixtures-update-pr.yaml was missing mongodb vars. And the
workflow was failing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryannikolaidis ryannikolaidis ryannikolaidis approved these changes

@scanny scanny scanny approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@potter-potter @ryannikolaidis @scanny