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

feat: Adding a js-langchain template draft #144

Merged
merged 23 commits into from
Aug 2, 2023
Merged

feat: Adding a js-langchain template draft #144

merged 23 commits into from
Aug 2, 2023

Conversation

mtrunkat
Copy link
Member

@mtrunkat mtrunkat commented May 4, 2023 •
edited
Loading

This is a first draft, could you take a look if this makes sense to you this way?

@github-actions github-actions bot added this to the 62nd sprint - Platform team milestone May 4, 2023
@github-actions github-actions bot added the t-platform Issues with this label are in the ownership of the platform team. label May 4, 2023
github-actions[bot]
github-actions bot reviewed May 4, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

github-actions[bot]
github-actions bot reviewed May 4, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

github-actions[bot]
github-actions bot reviewed May 5, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

github-actions[bot]
github-actions bot reviewed May 5, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

@mtrunkat mtrunkat added the adhoc Ad-hoc unplanned task added during the sprint. label May 10, 2023
@mtrunkat mtrunkat requested review from fnesveda and drobnikj May 25, 2023 14:08
@mtrunkat mtrunkat marked this pull request as ready for review May 25, 2023 15:17
mnmkng
mnmkng requested changes May 25, 2023
View reviewed changes
templates/js-langchain/.eslintrc Outdated Show resolved Hide resolved
templates/js-langchain/package.json Outdated Show resolved Hide resolved
drobnikj
drobnikj requested changes May 29, 2023
View reviewed changes
Copy link
Member

@drobnikj drobnikj left a comment

Choose a reason for hiding this comment

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

Looks good

templates/js-langchain/.actor/Dockerfile Show resolved Hide resolved
templates/js-langchain/.actor/actor.json Outdated Show resolved Hide resolved
templates/js-langchain/.actor/actor.json Outdated Show resolved Hide resolved
templates/js-langchain/package.json Outdated Show resolved Hide resolved
templates/js-langchain/src/main.js Outdated Show resolved Hide resolved
templates/js-langchain/src/main.js Outdated Show resolved Hide resolved
templates/js-langchain/src/main.js Show resolved Hide resolved
fnesveda
fnesveda requested changes May 29, 2023
View reviewed changes
Copy link
Member

@fnesveda fnesveda left a comment

Choose a reason for hiding this comment

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

I think the whole APIFY_API_TOKEN env var logic is too complicated and would turn people off, and it's not necessary 99% of the time.

And I would cache the index to a local key-value store rather than to the platform every time.

templates/js-langchain/src/main.js Outdated Show resolved Hide resolved
templates/js-langchain/src/main.js Outdated Show resolved Hide resolved
templates/js-langchain/src/vector_index_cache.js Outdated Show resolved Hide resolved
templates/js-langchain/.gitignore Outdated Show resolved Hide resolved
@fnesveda
Copy link
Member

fnesveda commented Jul 3, 2023

Ping @mtrunkat, any progress on this?

@drobnikj
Copy link
Member

@mtrunkat, maybe @jirimoravcik or @barjin can finish this.

@mtrunkat
Copy link
Member Author

Thanks, guys for your comments. A bit shame it took so long but I finally gave it another look.

@mtrunkat mtrunkat requested review from fnesveda, drobnikj and mnmkng July 15, 2023 09:25
@mtrunkat
Copy link
Member Author

@jbartadev I know you worked on the templates. Is there anything different now that I should reflect in this template?

@jbartadev
Copy link
Member

@jbartadev I know you worked on the templates. Is there anything different now that I should reflect in this template?

@mtrunkat Yes, there is.

  1. You need to add the template to the templates/manifest.json so it's available (and displayed) in the template library and in CLI. The position (order of how templates are displayed) is defined by this file. You need to define the category (language) and included technologies there too so it's displayed in the template detail in the console and on the web.

  2. We have a specific README structure to follow - there are keywords and patterns we use. Please see for example https://github.com/apify/actor-templates/tree/master/templates/js-start or an example structure in Notion.

Please let us know if you need any help with it.

fnesveda
fnesveda requested changes Jul 18, 2023
View reviewed changes
Copy link
Member

@fnesveda fnesveda left a comment

Choose a reason for hiding this comment

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

The caching logic is quite complicated, with the temporary files and everything, I think it could be simplified quite a bit.

templates/js-langchain/src/main.js Outdated Show resolved Hide resolved
templates/js-langchain/src/vector_index_cache.js Outdated Show resolved Hide resolved
templates/js-langchain/src/vector_index_cache.js Outdated Show resolved Hide resolved
@mtrunkat mtrunkat requested a review from fnesveda July 28, 2023 09:20
drobnikj
drobnikj approved these changes Jul 31, 2023
View reviewed changes
Copy link
Member

@drobnikj drobnikj left a comment

Choose a reason for hiding this comment

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

LGTM

fnesveda
fnesveda requested changes Aug 1, 2023
View reviewed changes
Copy link
Member

@fnesveda fnesveda left a comment

Choose a reason for hiding this comment

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

Looks good, last few notes

templates/manifest.json Outdated Show resolved Hide resolved
templates/js-langchain/package.json Outdated Show resolved Hide resolved
templates/js-langchain/src/main.js Outdated Show resolved Hide resolved
@mtrunkat mtrunkat merged commit 435a4b0 into master Aug 2, 2023
@mtrunkat mtrunkat deleted the feature/langchain_experiment branch August 2, 2023 11:23
@fnesveda fnesveda added the validated Issues that are resolved and their solutions fulfill the acceptance criteria. label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@fnesveda fnesveda fnesveda requested changes

@drobnikj drobnikj drobnikj approved these changes

@mnmkng mnmkng Awaiting requested review from mnmkng

Assignees

@mtrunkat mtrunkat

Labels
adhoc Ad-hoc unplanned task added during the sprint. t-platform Issues with this label are in the ownership of the platform team. validated Issues that are resolved and their solutions fulfill the acceptance criteria.
Projects
None yet
Milestone
69th sprint - Platform team
Development

Successfully merging this pull request may close these issues.
5 participants
@mtrunkat @fnesveda @drobnikj @jbartadev @mnmkng