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: update amenity_dataset with new hour columns #151

Merged
merged 10 commits into from
Jan 8, 2024
Merged

feat: update amenity_dataset with new hour columns #151

merged 10 commits into from
Jan 8, 2024

Conversation

Yuan325
Copy link
Collaborator

@Yuan325 Yuan325 commented Dec 27, 2023 •
edited
Loading

Update amenity_dataset to include 14 new column for start_hour and end_hour of each day.
Update postgres provider, cloudsql provider and tests.

@Yuan325 Yuan325 self-assigned this Dec 27, 2023
@Yuan325 Yuan325 requested a review from a team as a code owner December 27, 2023 15:45
@Yuan325 Yuan325 marked this pull request as draft December 27, 2023 15:45
@Yuan325 Yuan325 marked this pull request as ready for review December 28, 2023 15:19
@Yuan325 Yuan325 force-pushed the amenity-add-start-end-hours branch 3 times, most recently from d25f28e to f3a070d Compare December 28, 2023 15:26
@Yuan325 Yuan325 changed the title feat: update amenity_dataset and postgres provider feat: update amenity_dataset with new hour columns Dec 28, 2023
@Yuan325 Yuan325 force-pushed the amenity-add-start-end-hours branch 3 times, most recently from 35e1abc to 1837eb6 Compare December 28, 2023 15:49
@Yuan325 Yuan325 marked this pull request as draft December 29, 2023 17:27
@Yuan325 Yuan325 force-pushed the amenity-add-start-end-hours branch 2 times, most recently from 07d3391 to c2d968d Compare January 1, 2024 17:27
@Yuan325 Yuan325 marked this pull request as ready for review January 2, 2024 10:24
@Yuan325 Yuan325 requested a review from kurtisvg January 2, 2024 10:24
@Yuan325 Yuan325 force-pushed the amenity-add-start-end-hours branch 2 times, most recently from 4c79527 to ab42c48 Compare January 3, 2024 19:19
kurtisvg
kurtisvg requested changes Jan 4, 2024
View reviewed changes
retrieval_service/app/app_test.py Outdated Show resolved Hide resolved
retrieval_service/models/models.py Show resolved Hide resolved
retrieval_service/models/models.py
mode="before",
)
def replace_none(cls, v):
return v or None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this still neccesary with an optional field?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, within the csv dataset, these None value are empty. For example:
Sunday 9-10pm, 09:00:00, 22:00:00,,,,, (those None value is converted as just empty).
Hence, when running validation, if without replacing them with actual None, it will throw an error saying that the type is different.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the exact error:
Input should be in a valid time format, input is too short [type=time_parsing, input_value='', input_type=str]

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that's okay, but we probably need to update the retrieval service to say "closed" as opposed to None in the string it returns.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added a bug for this

@Yuan325 Yuan325 force-pushed the amenity-add-start-end-hours branch from ecf10ba to 58e5f2c Compare January 5, 2024 14:24
@Yuan325 Yuan325 requested a review from kurtisvg January 5, 2024 14:26
kurtisvg
kurtisvg approved these changes Jan 5, 2024
View reviewed changes
retrieval_service/datastore/providers/cloudsql_postgres.py Outdated Show resolved Hide resolved
retrieval_service/models/models.py
mode="before",
)
def replace_none(cls, v):
return v or None
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that's okay, but we probably need to update the retrieval service to say "closed" as opposed to None in the string it returns.

@Yuan325 Yuan325 force-pushed the amenity-add-start-end-hours branch from c578717 to 37bb58f Compare January 8, 2024 08:18
@Yuan325 Yuan325 merged commit 160d8df into main Jan 8, 2024
6 of 7 checks passed
@Yuan325 Yuan325 deleted the amenity-add-start-end-hours branch January 8, 2024 08:22
@github-actions github-actions bot mentioned this pull request May 31, 2024
Merged
Yuan325 pushed a commit that referenced this pull request Jul 1, 2024
@github-actions
chore(main): release 0.1.0 (#398)
00d8ddd 
🤖 I have created a release *beep* *boop*
---


## 1.0.0 (2024-06-27)


### Features

* Add "airports" dataset
([#8](#8))
([e566554](e566554))
* add "amenities" dataset
([#9](#9))
([74db3ff](74db3ff))
* add "flights" dataset
([#10](#10))
([eebd3ce](eebd3ce))
* add api endpoints for amenities
([#12](#12))
([bd1c411](bd1c411))
* add app_test cicd workflow
([#157](#157))
([51f0e51](51f0e51))
* add clean up instructions
([#99](#99))
([c9021b1](c9021b1))
* add cloud sql cloudbuild workflow
([#143](#143))
([3dd3444](3dd3444))
* add Cloud SQL MySQL as a datastore provider
([#415](#415))
([d3f43d7](d3f43d7))
* add cloudsql integration test
([#141](#141))
([764ffeb](764ffeb))
* add cloudsql provider
([#5](#5))
([32a063d](32a063d))
* add confirmation for ticket booking
([#407](#407))
([6987280](6987280))
* add cymbal air passenger policy
([#265](#265))
([199a67c](199a67c))
* add endpoint for policy
([#271](#271))
([397ca79](397ca79))
* Add Firestore provider
([#65](#65))
([0cbd8ed](0cbd8ed))
* add GET endpoint for airport
([#11](#11))
([d36fd29](d36fd29))
* add initial "flights" endpoints
([#17](#17))
([da374d9](da374d9))
* add initial readme
([#31](#31))
([cb6d68c](cb6d68c))
* add instructions file for cloudsql_postgres
([#93](#93))
([ca3d2c5](ca3d2c5))
* add instructions for alloydb
([#27](#27))
([a30d5d1](a30d5d1))
* add integration test for postgres
([#123](#123))
([0aeab54](0aeab54))
* add labels syncing config
([#40](#40))
([b27558f](b27558f))
* add mock for retrieval service test
([#126](#126))
([498c6d7](498c6d7))
* add orchestration interface
([#226](#226))
([573c04b](573c04b))
* Add Renovate Config
([#42](#42))
([40c221b](40c221b))
* add search airport tools to langchain agent
([#140](#140))
([94a5df3](94a5df3))
* add smoke test for function calling
([#199](#199))
([5eecedf](5eecedf))
* add test for database export
([#133](#133))
([b114186](b114186))
* Adding Spanner database provider for retrieval service.
([#316](#316))
([8990bc9](8990bc9))
* Create google sign in button and send id token with request
([#147](#147))
([c0a34a7](c0a34a7))
* Create individual user client session
([#137](#137))
([359e5d3](359e5d3))
* Create reset button to clear session cookies
([#152](#152))
([f008c04](f008c04))
* Create Tickets endpoints
([#160](#160))
([26ad5d9](26ad5d9))
* **doc:** Add Firestore setup instruction
([#87](#87))
([e6e6fb6](e6e6fb6))
* **docs:** add code of conduct
([#39](#39))
([0dee8dd](0dee8dd))
* **docs:** update docs
([#36](#36))
([e908e75](e908e75))
* **docs:** update gcloud track for deployment with VPC network
([#80](#80))
([84313c9](84313c9))
* implement signin signout
([#258](#258))
([88b6c83](88b6c83))
* increase font-size and make UI better
([#292](#292))
([c6dd0f6](c6dd0f6))
* inital prototype of extension
([a502f84](a502f84))
* minor UI improvements
([#275](#275))
([7e31ac3](7e31ac3))
* minor UI updates
([#279](#279))
([774703a](774703a))
* refactor demo frontend
([#15](#15))
([4245020](4245020))
* refresh UI
([#239](#239))
([0254ce8](0254ce8))
* replace October flights data
([#37](#37))
([c2c70a2](c2c70a2))
* Replace requests package with aiohttp
([#125](#125))
([4fc0d27](4fc0d27))
* update airport dataset and add search endpoint
([#14](#14))
([baabfe4](baabfe4))
* update amenity embedding to include amenity name
([#144](#144))
([3fe22ce](3fe22ce))
* update amenity_dataset with new hour columns
([#151](#151))
([160d8df](160d8df))
* update app in langchain demo
([#225](#225))
([7c463d9](7c463d9))
* update model to Gemini
([#158](#158))
([09959fd](09959fd))
* Update prompt and tools
([#34](#34))
([499f6a7](499f6a7))
* update service Dockerfile
([#19](#19))
([ea7edc7](ea7edc7))
* update to include user name in chat
([#249](#249))
([070b7e0](070b7e0))
* vertex ai function calling llm
([#188](#188))
([ab1aedc](ab1aedc))


### Bug Fixes

* add dependency
([#195](#195))
([65cc37c](65cc37c))
* add retrieval service service account role to doc
([#121](#121))
([8ea12c9](8ea12c9))
* **ci:** use script over args in all cloudbuild
([#307](#307))
([da8d3f6](da8d3f6))
* **ci:** use script over args in cloudbuild syntax
([#306](#306))
([b5aa667](b5aa667))
* Do not pass None values to session.get()
([#136](#136))
([5d9cbbc](5d9cbbc))
* **docs:** fixed broken links to demo apps
([#132](#132))
([4955cd0](4955cd0))
* **firestore:** Add ID to all documents in Firestore provider
([#94](#94))
([1a02328](1a02328))
* Fix closed connection issue on reset button
([#175](#175))
([21607ec](21607ec))
* Fix get_agent() uuid comparison
([#200](#200))
([bc68633](bc68633))
* fix the refresh icon hiding behind Google banner
([#247](#247))
([1d858ba](1d858ba))
* **langchain_tools_demo:** Add ID Token credential flow for GCE
([#198](#198))
([ed9b6c2](ed9b6c2))
* **langchain_tools_demo:** fix agent concurrency between restarts
([#194](#194))
([2584154](2584154))
* **langchain_tools_demo:** use relative paths for resources
([#192](#192))
([6f8ae0d](6f8ae0d))
* Make `clientId `optional in config.yml
([#207](#207))
([3914d8c](3914d8c))
* missing Client ID
([#196](#196))
([960f6b1](960f6b1))
* sign out when user token invalid
([#329](#329))
([2ec915b](2ec915b))
* strip query params in loginUri
([#425](#425))
([41f3bb7](41f3bb7))
* update AlloyDB instruction order
([#92](#92))
([334257c](334257c))
* update alloydb.md to remove extra trailing space
([#46](#46))
([7f97d33](7f97d33))
* update embeddings and pin model
([#124](#124))
([5842c08](5842c08))
* update renovate config
([#378](#378))
([2ec52ce](2ec52ce))
* update search airport tool
([#148](#148))
([d4b36e9](d4b36e9))
* Use idiomatic python for conditional
([#149](#149))
([18e5e9d](18e5e9d))
* use lazy refresh for AlloyDB and Cloud SQL Connector
([#429](#429))
([c73484d](c73484d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kurtisvg kurtisvg kurtisvg approved these changes

Assignees

@Yuan325 Yuan325

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Yuan325 @kurtisvg