Make decorators work with the new SDK. #115
Conversation
js-ts
commented
Jan 30, 2025
js-ts
commented
- update rendering of run.py
- switch from component.yaml to configs directory with json files
- update rendering of .env
- added .gitignore
- Changed the name of the directory from agent_pkgs to naptha_modules
naptha_sdk/client/naptha.py
Outdated
| } | ||
| logger.info(f"Registering Agent {agent_config}") | ||
| agent = await self.hub.create_or_update_agent(agent_config) | ||
| logger.info("Registering Agent %s", agent_config) |
There was a problem hiding this comment.
why move away from f string?
There was a problem hiding this comment.
@js-ts This is still unresolved. You have still not reverted the changes to f string in several places. We should be consistent.
| if hasattr(deployment, subdeployment) and getattr(deployment, subdeployment): | ||
| for submodule in getattr(deployment, subdeployment): | ||
| if "module_type" not in submodule.module: | ||
| submodule.module["module_type"] = mod_type | ||
| modules.append(submodule.module) | ||
| else: |
There was a problem hiding this comment.
which case do you use for the decorators. we talked about removing this if statement and having only one case. was that possible?
There was a problem hiding this comment.
@js-ts i still see two cases if not decorator: and else:
why do we need both of these cases?
There was a problem hiding this comment.
These are needed, The two cases support different workflows.
There was a problem hiding this comment.
can you explain what the two different cases are needed for?
| else: | ||
| logger.warning(f"No .env file found in {source_dir}") | ||
|
|
||
| def copy_configs_directory(source_dir, package_name): |
There was a problem hiding this comment.
where are these copied from?
There was a problem hiding this comment.
from this path crew-ai/stock_analysis. where you run poetry run python src/stock_analysis/crew.py
There was a problem hiding this comment.
So do you create these configs in crew-ai/dtock_analysis and then copy them to a new dir? Why not just create them in the new dir?
There was a problem hiding this comment.
@js-ts can you clarify on this? why is the configs dir created in crew-ai/stock_analysis initially? Why not create directly at naptha_modules/agent_module?
There was a problem hiding this comment.
I created them manually considering that there are several different cases which a generated config couldn't handle.
There was a problem hiding this comment.
@js-ts can you point me to where the files in configs are created?
There was a problem hiding this comment.
Oh you mean you didn't create them using code? That isn't ideal. Why don't you think code to generate the configs dir will work?
There was a problem hiding this comment.
Do you want sample configs like this to be generated which would be just deployment.json and llm_configs.json:
https://github.com/NapthaAI/simple_chat_agent/tree/main/simple_chat_agent/configs
and then the user modifies them to add in things later?
https://github.com/NapthaAI/multiagent_chat/tree/main/multiagent_chat/configs
I thought that this approach lacked relevant generation.
Or
Do you want the configs to be entirely generated based on what ever information we can get from the crew.py file? IDK if sufficient information can be extracted this way rather than having them define it themselves
Do you know a better way of generating them without the issues?
There was a problem hiding this comment.
@js-ts Yes exactly just create the deployment.json and llm_configs.json.
I think it will look pretty much exactly like this https://github.com/NapthaAI/simple_chat_agent/blob/main/simple_chat_agent/configs/deployment.json
You just need to add the correct module name.
You can just leave the persona field blank.
| logger.info("Module URL: %s", module_url) | ||
| logger.info( | ||
| "IPFS Hash: %s. You can download it from " | ||
| "http://ipfs-gateway.naptha.work/ipfs/%s", |
There was a problem hiding this comment.
is this correct? i thought it was ipfs.naptha.work not ipfs-gateway.naptha.work
@moarshy
| with open(env_example_path, 'w') as env_file: | ||
| env_file.write('OPENAI_API_KEY=\n') | ||
| env_file.write(env_content) |
There was a problem hiding this comment.
@js-ts why do we need this, if .env is copied from crew-ai/stock_analysis?
There was a problem hiding this comment.
This was previously there and I just aded new keys.
There was a problem hiding this comment.
@js-ts ok so just to confirm: you prefer to copy the .env from crewai-examples/stock-analysis rather than using this function?
There was a problem hiding this comment.
The copied env file doesn't get pushed, I thought it would better to keep it if someone else wants to use the module.
